Fix S3 dispatch, multi-package loading, and export processing#14
Merged
Merged
Conversation
load_fast() replicated R's namespace machinery but omitted several pieces that real packages and multi-package sessions depend on: - S3 methods declared via S3method() were never registered, so the namespace's .__S3MethodsTable__. stayed empty and S3 dispatch fell through to *.default / the base method (even for print/format). Now calls registerS3methods() like base::loadNamespace(), and rebuilds the table on incremental reloads so re-sourced methods take effect. - The namespace info was missing the `lazydata` field, so `pkg::name` for any name not in exports threw "object 'lazydata' not found" (the documented multi-package bug). Now seeds `lazydata` and `nativeRoutines` at namespace creation, matching makeNamespace(). - Only export() was processed. exportPattern(), exportClasses(), and exportMethods() were ignored, so exportPattern packages exported nothing (pkg::fn failed) and cross-package importClassesFrom() / importMethodsFrom() errored. Now computes exports the way loadNamespace() does, including the internal-name stoplist. - .onAttach was never run. Now runs on full load after attach. Adds an S3 fixture to devpackage and test stages covering S3 dispatch (single- and cross-package), incremental S3 refresh, exportPattern / exportClasses / exportMethods, `::` resolution, and .onAttach. Suite is 259 passing. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0133UhzJyq9xA2BBSA6B1LTd
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
load_fast()reimplements R's namespace machinery, but several pieces that real packages and multi-package sessions depend on were missing. I reproduced each issue against R, fixed it to mirrorbase::loadNamespace(), and verified from both the source harness and an installed package namespace..__S3MethodsTable__.stayed empty; S3 dispatch fell through to*.default/ the base method even forprint/format, for any method declared withS3method()and not exported by name (the modern default)registerS3methods()likeloadNamespace(); rebuild the table on incremental reloads so re-sourced methods take effectlazydatafield missing (documented multi-package bug)pkg::namefor any name not inexportsthrewobject 'lazydata' not foundlazydata+nativeRoutinesat namespace creation, matchingmakeNamespace()exportPattern()ignored (common trigger for the above)exportPattern("^[^.]")exported nothing, sopkg::fnfailedloadNamespace(), with the internal-name stoplistexportClasses()/exportMethods()ignoredimportClassesFrom()/importMethodsFrom()errored hard.onAttachnever rundevtools::load_all()Tests
devpackage/(s3_classes.R+S3method()/export()directives + testthat blocks)..onAttachin Stage 4l, and a new Stage 5 for cross-package fidelity (S3 across packages,exportPatternincl. the.stoplist,exportClasses/exportMethods,pkg::resolution).R CMD INSTALL.Docs
AGENTS.md: added namespace-machinery notes (S3 registration, export processing + stoplist,lazydata,.onAttach) and updated the test-stage/fixture descriptions.TECHNICAL_DEBT.md: recorded the fidelity improvements and the "reload doesn't re-read NAMESPACE" tradeoff.Notes
NAMESPACE, so adding anexport()/S3method()/import still needsfull = TRUE(consistent with the existing treatment of imports; documented).🤖 Generated with Claude Code
https://claude.ai/code/session_0133UhzJyq9xA2BBSA6B1LTd
Generated by Claude Code