[Scons-users] JavaCommon target prediction bug

[Matthew Marinets]

I first encountered this bug a few months ago, but I found a couple of easy workarounds and patches and just forgot about it. Then I realized that there's a whole series of Java target-prediction-related bugs on Github, and tried to see if I could isolate some of these patches I made for you all.

The bug:
Implementing an abstract class in the constructor for another abstract class that is about to be implemented messes up the anonymous inner class stack. Confusing, but just look at the example.

*I'm using Java 1.8, scons 3.0.1, and Python 3.6.5

Minimal case:
package com.Matthew;

public class AnonDemo {

    public static void main(String[] args) {
        new AnonDemo().execute();
    }

    public void execute() {
        Foo bar = new Foo(new Foo() {
            @Override
            public int getX() { return this.x; }
        }) {
            @Override
            public int getX() { return this.x; }
        };
    }

    public abstract class Foo {
        public int x;
        public abstract int getX();

        public Foo(Foo f) {
            this.x = f.x;
        }

        public Foo() {}
    }
}

All that's happening in AnonDemo.execute() is that Foo's constructor is called and it is implemented twice. The first time, I use the default constructor, and the second time I use the previous instantiation as an argument in a copy constructor (it might not be called that; I learned the jargon for C++, and I'm not sure if it's the same in Java).

The resulting .class files are:
com/Matthew/AnonDemo$1.class
com/Matthew/AnonDemo$2.class
com/Matthew/AnonDemo$Foo.class
com/Matthew/AnonDemo.class

The class files SCons predicts are:
com/Matthew/AnonDemo$1.class
com/Matthew/AnonDemo$1$1.class
com/Matthew/AnonDemo$Foo.class
com/Matthew/AnonDemo.class

Notes:

  *   This bug only arises when a class is implemented in the constructor to another implemented class.
  *   If further classes are implemented later in the java file, SCons's anonymous inner class counter is locked in a state of having an extra number on the stack
     *   E.g. "$1$2, $1$3..." instead of going back to "$2, $3..."
  *   This bug comes from the particular way that JavaCommon.py deals with '(' tokens in anonState.
  *   While this might seem like an edge case, this comes up a lot in my company's code base when dealing with forms (implement a JButton that takes an implementation for an AbstractAction, etc)

The detailed cause:
I already have a fix for this particular case, and it seems to work well for our project at work. You can find a link to the branch on github at the bottom of this email.

The general idea is to adjust how JavaCommon deals with anonymous inner classes. Currently, the SCons module in charge of parsing Java files, SCons/Tool/JavaCommon.py, is essentially a giant state machine. It starts in OuterState, and each new state is put on a stack. Changing state involves either popping the current state off the stack and returning to the previous one, or adding a new one on the stack that has a reference to the old one. Let's look at how it parses the execute() method in the example above:


Foo bar = new Foo(new Foo() {
    @Override
    public int getX() { return this.x; }
}) {
    @Override
    public int getX() { return this.x; }
};
Tokens (just the first line):
new, Foo, (, new, Foo, (, ), {,

Parser actions:
I will notate the state as <state> <position on stack>. So OuterState is always "OuterState 0", and if it creates a anon state that then adds a skip state, the skip state would be "SkipState 2".

<start> -- the state machine starts in OuterState 0.
new - Create AnonState 1 and skip the next token (SkipState 2)
Foo - skipped; return to AnonState 1
( - AnonState 1's brace level +1
new - because AnonState 1's brace level is non-zero, skip the next token (SkipState 3) and return to a new AnonState (AnonState 2)
() - brace level +1, -1; no net effect
{ - this is where things break. AnonState 2:

  *   Calls OuterState.addAnonClass(), which:
     *   Adds class$1 to the list of outputs
     *   Adds a new anon class counter to a stack, so the next anon class would be class$1$1
     *   Under normal circumstances, AnonState would return to OuterState, and this new slot would then be popped. However:
  *   AnonState 2 then pops itself off the stack and gets the resulting state to parse the same token. So:
  *   AnonState 1 parses the { token. But OuterState didn't have a chance to pop the anon state counter, so this anon state gets added as class$1$1 when it should be class$2

My solution:
I added a new state, which I called ScopeState. It acts much the same as OuterState, except it only exists for one curly-brace scope. That is, if the first token it parses isn't {, then it pops off the stack immediately, otherwise it works like OuterState until its curly-brace level drops back to 0. The only entry point for ScopeState is when an AnonState comes across the { token, adds to the anon state counter, and has to return to something - it returns a ScopeState of the same stack position as itself.

So in the above example, when AnonState 2 pops itself off the stack, it returns to ScopeState 2, which blocks AnonState 1 from adding to the anon state counter until the anon stack has been properly formatted.

That was very hard to describe, sorry if you couldn't follow. Maybe it'll be easier if you just look at the code.

The one caveat I found was that this actually broke some SCons unit tests in JavaCommonTests.py, specifically test_anonymous_clases_with_parentheses. This test doesn't run on 1.8, and I'm unfamiliar with how other Java versions act differently, so I just added a switch in JavaCommon that only runs my changes if the version is in a list (called ScopeStateVersions). For now, only 1.8 is in that list, but someone else can add to it if they verify it works.

Github: https://github.com/MatthewMarinets/scons/tree/JavaParserFix
I haven't read the SCons submit guidelines (: P), so I'll hold off on actually submitting a pull request until I've had a chance to look at that.

-Matthew


-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://pairlist4.pair.net/pipermail/scons-users/attachments/20180628/99006dd1/attachment.html>