GraphDependencies and completion

[Vicente-Arturo Romero-Zaldivar]

On 10/16/2015 02:17 PM, Maurizio Cimadamore wrote:
> Would a delegation model work for problem 2? I.e. the dependencies 
> completer could be a 'spy' completer that did its own bit of logic and 
> then delegated to the completer originally installed (which could be a 
> real completer or the null completer).

I also thought about this, but IMO the model could be more general. I 
mean it doesn't seem to be necessary to stablish an order between the 
dependencies completer and the "original" completer. It seems to me like 
they can be executed in any order thus effectively working as "event 
listeners". This could imply having a list, or set if you want to 
enforce uniqueness, of completers instead of only one completer.

Vicente

>
> Maurizio
>
> On 16/10/15 12:24, Andreas Lundblad wrote:
>> Widening the audience of this discussion. Jan and Maurizio, forgive 
>> the overlap with previous email.
>>
>> I've looked into the Dependencies class and tried to figure out how 
>> it can be adjusted to cover the requirements of sjavac (with the help 
>> of Jan and Maurizio). A summary follows.
>>
>> Currently the Dependencies mechanism works as follows: When a symbol 
>> is about to be completed, it's pushed onto a stack and when 
>> completion finishes the stack is popped. If a symbol A is pushed, and 
>> then symbol B is pushed on top of A, it means that the completion of 
>> A triggered completion of B and by that we conclude that A depends on 
>> B. To hook into the completion mechanism the Dependencies class 
>> replaces the null completer with itself.
>>
>> There are currently two issues with this approach.
>>
>> 1) If symbol A triggers completion of symbol B, but B happens to 
>> already be completed the dependency from A to B may go unnoticed.
>>
>> 2) When the Dependencies installs itself as a completer for the 
>> symbol, it *might* overwrite an important completer. Right now it 
>> seems like this doesn't happen, but it's fragile.
>>
>> Problem 1 can be solved by always calling complete, even for the null 
>> completer. (I will benchmark carefully to make sure it doesn't cause 
>> a performance regression.)
>>
>> Problem 2 is trickier. The most robust alternative would be to move 
>> the Dependencies.push/pop logic into Symbol.complete. Symbol doesn't 
>> have a reference to the context however, so getting hold of 
>> Dependencies is problematic. Another alternative would be to move the 
>> functionality over to the completer. It can be done without spreading 
>> the logic across all completers by making Completer an abstract class 
>> that takes care of this. I've tried this and it is possible but it 
>> gets quite complicated since for instance even the null completer 
>> needs to reference the context, and Symbol.complete needs a reference 
>> to the null completer.
>>
>> Another option that has struck me is to let Symbol keep track of the 
>> dependencies by having a 'private static Symbol currentlyCompleted'. 
>> The complete method would then look something like
>>
>>      if (currentlyCompleted != null)
>>          currentlyCompleted.deps.add(this);
>>      Symbol prev = currentlyCompleted;
>>      currentlyCompleted = this;
>>      c.complete(this);
>>      currentlyCompleted = prev;
>>
>> Unfortunately this solution would actually require currentlyCompleted 
>> to be something like a ThreadLocal, which is not the javac-idiomatic 
>> way of solving per javac instance data, so I assume this is a no-go?
>>
>> So, since I've already spent a lot of time on this, I'm now thinking 
>> of ignoring issue 2 for the time being. If anyone sees a solution, 
>> please let me know. (Should the current solution fall apart, my plan 
>> B is to go back to analyzing the AST.)
>>
>> -- Andreas
>