Make trace() work for functions whose S3 class has multiple strings#262
Open
hadley wants to merge 1 commit into
Open
Make trace() work for functions whose S3 class has multiple strings#262hadley wants to merge 1 commit into
hadley wants to merge 1 commit into
Conversation
trace() failed for any function with a multi-string S3 class attribute,
e.g. S7 generics and methods (class c("S7_method", "function",
"S7_object")), with
'length = 3' in coercion to 'logical(1)'
because the class vector was passed where a single class name was
expected. Reported via RConsortium/S7#584.
In methods:::.TraceWithMethods:
* Use .class1() rather than class() to derive the "<class>WithTrace"
class name, so the trace class extends the (S4 view of) the most
specific class instead of setClass() receiving a vector of names.
In methods:::.initTraceable:
* Use .class1() likewise for the old class lookup.
* Skip the `as(.Object, oldClass) <- def` slot copy when oldClass is
virtual, i.e. an S3 class registered by setOldClass(). Such classes
have no slots to copy beyond .Data, and for them the coercion
replaced .Object wholesale with def, losing the trace wrapper.
* Extract the data part with slot(def, ".Data") instead of def@.Data:
since R 4.3, `@` on non-S4 objects dispatches on S3 methods, so a
package method such as S7's @.S7_object intercepted the call and
errored.
Tracing an S7 *method* now works end to end with no changes to S7.
Tracing an S7 *generic* additionally needs S7's C-level change
(RConsortium/S7#695): the traced binding is an S4 object of class
"S7_genericWithTrace" whose link to "S7_generic" exists only in the S4
class table, so the Rf_inherits(generic, "S7_generic") check in S7's
method-dispatch.c cannot see it and S7 must unwrap the "original"
attribute itself. That is inherent to how trace() wraps functions and
not fixable in R: an S4 object's class attribute is its S4 class, and
making inherits() consult S4 inheritance would be far more invasive.
S7's R-level workaround (pre-registering the WithTrace classes) remains
useful on R versions without this fix and for property access and
printing of traced generics.
Adds a regression test in tests/classes-methods.R and a NEWS entry.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
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
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.
trace() failed for any function with a multi-string S3 class attribute, e.g. S7 generics and methods (class
c("S7_method", "function", "S7_object")), with'length = 3' in coercion to 'logical(1)'because the class vector was passed where a single class name was expected. Reported via RConsortium/S7#584.In
methods:::.TraceWithMethods:.class1()rather thanclass()to derive the"<class>WithTrace"class name, so the trace class extends the (S4 view of) the most specific class instead ofsetClass()receiving a vector of names.In
methods:::.initTraceable:Use
.class1()likewise for the old class lookup.Skip the
as(.Object, oldClass) <- defslot copy when oldClass is virtual, i.e. an S3 class registered by setOldClass(). Such classes have no slots to copy beyond .Data, and for them the coercion replaced .Object wholesale with def, losing the trace wrapper.Extract the data part with
slot(def, ".Data")instead ofdef@.Data: since R 4.3,@on non-S4 objects dispatches on S3 methods, so a package method such as S7's@.S7_objectintercepted the call and errored.Tracing an S7 method now works end to end with no changes to S7.
Tracing an S7 generic additionally needs S7's C-level change (RConsortium/S7#695): the traced binding is an S4 object of class
"S7_genericWithTrace"whose link to"S7_generic"exists only in the S4 class table, so theRf_inherits(generic, "S7_generic")check in S7'smethod-dispatch.ccannot see it and S7 must unwrap the "original" attribute itself. That is inherent to howtrace()wraps functions and not fixable in R: an S4 object's class attribute is its S4 class, and makinginherits()consult S4 inheritance would be far more invasive. S7's R-level workaround (pre-registering the WithTrace classes) remains useful on R versions without this fix and for property access and printing of traced generics.Adds a regression test in
tests/classes-methods.Rand aNEWSentry.