LeonMatthes / Autocompletion

A modern Autocompletion for your Squeak image
8 stars 3 forks source link

Proposal: Instance guessing #32

Open LinqLover opened 5 years ago

LinqLover commented 5 years ago

Type guessing is a very nice feature, but I think in some cases, information about the actual accessed instance could be useful for further type guessing.

There are many quick methods in Squeak (accessors or magic number returns) which are very fast and side-effect-free. We could guess the instance of the last typed expression in some cases and then perform a possible quick method, to "guess" the type of its return value.

One popular scenario might be Autocompletion in Inspector: Do Morph new inspect, then type bounds origin sqr. Currently, there is no suggestion for sqrt, but as Rectangle>>#bounds is quick, it would be possible to find out the type of bounds origin and so to suggest sqrt. It would be even possible to suggest self bounds origin x sqrt etc.

Possible sources for instance guessing

Proposed design changes

Questions

These are only some thoughts by someone who is not deep into the implementation, so I would be very happy about your feedback! So long, best success for your thesis, @MrModder :)

LeonMatthes commented 5 years ago

This suggestion taps into the same idea as #17 (debugger type info).

However, you seem to be proposing 2 changes with this issue:

Regarding Analysing unknown methods

If I understand you correctly, you want to perform accessors or other small methods on existing objects in the system.

This is problematic, not because of performance concerns, but rather because of possible side effects. You never know whether performing a method will result in side effects. And producing side effects on the objects we are trying to provide type guessing for is an absolute no-go!

Even if you try to analyse the method to find out, whether the method does have side-effects, you cannot be certain about this. Meta-Programming could have been used to perform any kind of nasty message rerouting.

However, it would be possible to analyse the code of the method with conventional means to try to find the returned type. The existing ECContext is responsible for this and it simply parses the code using the SHParser80 and analyses the ranges (primitive but mostly effective)

Regarding using objects to enhance type guessing

This should be the focus of this issue. It is important not to mix up two concerns here!

In certain scenarios (Debugger, Workspace, etc.), it is possible, that some of the objects that type guessing is actually used for are available for further information. This can be very helpful to exactly determine the types and even the exact instances of instance variables.

However, your proposed method guessInstanceOf: is not really fitting for the architecture. The guess... methods in ECContext are all responsible for using the method source to guess the class of the last range. If I understand you correctly, you want to enhance this type information by not just guessing for classes, but specific instances. So we don't need an extra guessInstanceOf: method, we need to expand the existing methods to be able to work with both classes and exact instances, if they are available. For example guessArgument: could actually ask the model for the exact object if it's known.

This could considerably enhance type guessing for longer accessor chains (whether they are good coding practice is a different discussion). It could also remove uncertainty, if polymorphism is used, because we would actually know the used subclass and not just the superclass that is expected by e.g. the accessors.

Answering your questions

guessUnaryAccessorClass: is responsible for allowing type guessing on instance variables even if you use accessors. Part of the Autocompletion is to promote good coding style, like using accessors. When generating accessors and filling out the parameter name in the setter with a<Typename>, the Autocompletion will give you type suggestions for that pair of accessors. If #33 were to be implemented, it would probably not be necessary anymore.

instance guessing is not quite the thing we're planning to do here. Maybe rather call it "instance/object supported type guessing" or something like that.

guessTypeForName: and guessObjectForName: are certainly different, yet related concerns. guessObjectForName: of course only works if you have an instance available. So you can't just redirect guessTypeForName: to it. Sometimes you just don't have an instance available, but may still know the class of it. In any case, if we want to provide object supported type guessing, we would still need to support class based type guessing, as that is the only way available in the class browser.

I hope I understood your intention correctly and this feedback is good food for thought. If not, please let me know.

LeonMatthes commented 5 years ago

Concrete steps that would be a good implementation strategy:

Add instance variables to the ECContext to support type info from Smalltalk-context or something similar.

Enhance the existing guess... methods to use available instances as the first way to guess the type.

During this step, it's necessary to implement a custom TypeInfo object, which can take either a class or an instance as the basis of type information.

LinqLover commented 5 years ago

Hi, many thanks for your detailed answer!

Let me start from the end:

Add instance variables to the ECContext to support type info from Smalltalk-context or something similar.

Did you read my PR #29? I think this implements just what you wrote.

Regarding Analysing unknown methods

If I understand you correctly, you want to perform accessors or other small methods on existing objects in the system.

This is problematic, not because of performance concerns, but rather because of possible side effects. You never know whether performing a method will result in side effects. And producing side effects on the objects we are trying to provide type guessing for is an absolute no-go!

Even if you try to analyse the method to find out, whether the method does have side-effects, you cannot be certain about this. Meta-Programming could have been used to perform any kind of nasty message rerouting.

What I wanted to propose was only to evaluate quick methods (see CompiledMethod>>#isQuick). If I understand their intention correctly, only methods that include nothing more than a quick return statement will return true for this test. Such methods thus cannot have side-effects.

More complex methods like the following would not be covered by this proposal:

foo
    ^ foo ifNil: [foo := self generateFoo]

But aside from the special case of quick methods, I totally agree that scanning each method for possible side-effects, in order to possibly perform it, would be overengineering, respecting the fact that method chains should be used rarely following LoD (Law of Demeter).

Regarding using objects to enhance type guessing

I'm not sure why you would not like to implement #guessInstanceOf:. Even when we have no current execution context, there are some concrete instances to deal with, such as literals or global vars. Typing World extent x sqrt could be completely assisted by instance-guessing. Wouldn't it provide a better overview to separate this two concerns (analyzing classes vs. instances)?

If I understand you correctly, you want to enhance this type information by not just guessing for classes, but specific instances.

Exactly :)

So we don't need an extra guessInstanceOf: method, we need to expand the existing methods to be able to work with both classes and exact instances, if they are available. For example guessArgument: could actually ask the model for the exact object if it's known.

Then #guessArgument: should return an instance, not a class, so that we can use the result to perform further quick methods on it (see above). Can you see my point?

One other problem with #guessObjectForName: would be the inability of the proposed protocol to differentiate between the guessed instance nil and the case "guessing not possible". Maybe let's better call it #guessObjectForName:ifPositive: or something like that. Default implementation of this method then could be ^self, and #guessTypeForName: could return nil. If I am not mistaken, currently each implementor of #guessTypeForName: knows the concrete object so it could directly implement #guessObjectForName:(ifPositive:).

In any case, if we want to provide object supported type guessing, we would still need to support class based type guessing, as that is the only way available in the class browser.

Such a class could override #guessTypeForName:.

Please also see my other answer in #33.