[BUG] Objects as methods break method search

Steps to reproduce:

Debug TestObjectsAsMethods >> #testAddNumbers, for example. Keep the debugger open.

Search for senders of any literal in your image, for example, "senders of #foo".

Expected behavior:

A MessageTrace with the calling methods opens as usual.

Actual behavior:

In SystemNavigation >> #allCallsOn:fromBehaviors:sorted:, an exception is raised:

MessageNotUnderstood: ObjectsAsMethodsExample>>hasLiteral:scanForSpecial:.

Considerations:

There are further bugs, for example, browse implementors of #foo, which will give you an MNU for AbstractObjectsAsMethod>>isDeprecated.

I see three possible solution approaches:

Approach A: Define an abstract superclass of all "objects as methods" classes, for example in AbstractObjectsAsMethod. Add an implementation of #hasLiteral:scanForSpecial: there, it might be sufficient to return false.

Pro: We can assure each value in a method dictionary actually understands a minimum protocol and do not need to add safety checks at other places such as SystemNavigation.

Con: We increase the complexity of implementing an object as method. By requiring a certain superclass, it is not possible for an existing class to accept the OaM responsibility.

Con: The number of methods to implement in this superclass might be large and unclear.

Approach B: In SystemNavigation >> #allCallsOn:fromBehaviors:sorted:, check that method responds to #hasLiteral:scanForSpecial: before sending the message, or catch any DNUs via #ifError: or so. In CodeHolder >> #formattedLabel:forSelector:inClass:, we would need to wrap the call to #isDeprecated the same way.

Con: There is possibly a very large number of clients of method dictionaries. It will be a mess to secure them all.

Note also how WrappedBreakpoint and TestCoverage are implemented. They keep a reference to the actual CompiledMethod they replace and forward all messages not understood to this method. There is duplication among these two classes, maybe we should refactor this as well.

However, the forwarding approach cannot work if there is no wrapped method available.

So Approach C would be: Should we maybe provide some kind of "null method object" we could forward all unknown messages to? Each OaM implementor that is not a wrapper then would need to implement the following method:

doesNotUnderstand: aMessage

^ aMessage sendTo: CompiledMethod nullMethod

Pro: Very easy to implement for both users of method dictionaries and implementors of OaM.

Con: It is not possible for an existing class that already forwards messages via #doesNotUnderstand: to accept the OaM responsibility. But this appears rather a hypothetic problem to me, as you could always decompose this responsibility ...

(For illustration, the probably cheapest fix of the concrete bugs mentioned above might be this:

AbstractObjectsAsMethod >> doesNotUnderstand: aMessage

^ aMessage sendTo: thisContext method

But this is the drawback of such a workaround:

)

So, which way would you go? To me, approach C sounds most useful, as we could actually provide a tweaked instance of CompiledMethod instead of reimplementing the protocol in another class (and having to maintain it).

Do we already have any constructor that could work as a method null object? Something like "Compiler new compiledMethodFor: '' in: nil to: nil notifying: nil ifFail: nil"? But this one has also two literals.

Best,

Christoph

Am 24.02.2020 13:42:58 schrieb Thiede, Christoph <[hidden email]>:

Steps to reproduce:

Debug TestObjectsAsMethods >> #testAddNumbers, for example. Keep the debugger open.

Search for senders of any literal in your image, for example, "senders of #foo".

Expected behavior:

A MessageTrace with the calling methods opens as usual.

Actual behavior:

In SystemNavigation >> #allCallsOn:fromBehaviors:sorted:, an exception is raised:

MessageNotUnderstood: ObjectsAsMethodsExample>>hasLiteral:scanForSpecial:.

Considerations:

There are further bugs, for example, browse implementors of #foo, which will give you an MNU for AbstractObjectsAsMethod>>isDeprecated.

I see three possible solution approaches:

Approach A: Define an abstract superclass of all "objects as methods" classes, for example in AbstractObjectsAsMethod. Add an implementation of #hasLiteral:scanForSpecial: there, it might be sufficient to return false.

Pro: We can assure each value in a method dictionary actually understands a minimum protocol and do not need to add safety checks at other places such as SystemNavigation.

Con: We increase the complexity of implementing an object as method. By requiring a certain superclass, it is not possible for an existing class to accept the OaM responsibility.

Con: The number of methods to implement in this superclass might be large and unclear.

Approach B: In SystemNavigation >> #allCallsOn:fromBehaviors:sorted:, check that method responds to #hasLiteral:scanForSpecial: before sending the message, or catch any DNUs via #ifError: or so. In CodeHolder >> #formattedLabel:forSelector:inClass:, we would need to wrap the call to #isDeprecated the same way.

Con: There is possibly a very large number of clients of method dictionaries. It will be a mess to secure them all.

Note also how WrappedBreakpoint and TestCoverage are implemented. They keep a reference to the actual CompiledMethod they replace and forward all messages not understood to this method. There is duplication among these two classes, maybe we should refactor this as well.

However, the forwarding approach cannot work if there is no wrapped method available.

So Approach C would be: Should we maybe provide some kind of "null method object" we could forward all unknown messages to? Each OaM implementor that is not a wrapper then would need to implement the following method:

doesNotUnderstand: aMessage

^ aMessage sendTo: CompiledMethod nullMethod

Pro: Very easy to implement for both users of method dictionaries and implementors of OaM.

Con: It is not possible for an existing class that already forwards messages via #doesNotUnderstand: to accept the OaM responsibility. But this appears rather a hypothetic problem to me, as you could always decompose this responsibility ...

(For illustration, the probably cheapest fix of the concrete bugs mentioned above might be this:

AbstractObjectsAsMethod >> doesNotUnderstand: aMessage

^ aMessage sendTo: thisContext method

But this is the drawback of such a workaround:

)

So, which way would you go? To me, approach C sounds most useful, as we could actually provide a tweaked instance of CompiledMethod instead of reimplementing the protocol in another class (and having to maintain it).

Do we already have any constructor that could work as a method null object? Something like "Compiler new compiledMethodFor: '' in: nil to: nil notifying: nil ifFail: nil"? But this one has also two literals.

Best,

Christoph

Hi Marcel,

> Every change to the interface of CompiledCode (etc.) can make all existing "method objects" incompatible.