java.lang.NullPointerException: Cannot read field "declaringScope" because "outerLocalVariable" is null after upgrading from 2.11 to 2.12.1 #10065

Activity

1. niloc132 commented on Dec 17, 2024

   

   niloc132

   on Dec 17, 2024

   Member

   More actions

   I've put up a snapshot build to test with - this is 2.12.1 plus a single try/catch in AstVisitor.visit(LambdaExpression,BlockScope) to dump the context of the error. It looks like a few others of those are missing, I'll add them also for the next release. Please try GWT version 2.12.2-10065-lambda-test-SNAPSHOT from https://repo.vertispan.com/gwt-snapshot/, it should give us a little more context, and possible offer a workaround.

   Reading pushLambdaExpressionLocalsIntoMethodScope where this failed, it appears that we're iterating the syntheticArguments array and one of those entries had a actualOuterLocalVariable that was null. Synthetic arguments are usually something like an "effectively final" variable in the method where the lambda was declared - readable, but not writable. The compiler wraps those up as "constructor arguments", same as if this was an inner class of some kind, and then the method can access them as fields of the inner type.

   The syntheticArguments array was not created by GWT, but passed in from LambdaExpression.outerLocalVariables - so something fishy is happening there. Either we have a new kind of outer local variable that a lambda effectively closes over (but doesn't correspond to a real variable) and GWT is missing support for this, or we've hit a bug in JDT?

   There is another field in SyntheticArgumentBinding, perhaps to look for a matching field instead of a local? That field isn't new though, both fields have been there for 22 years...
   https://github.com/eclipse-jdt/eclipse.jdt.core/blame/master/org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/lookup/SyntheticArgumentBinding.java
2. koendecock commented on Dec 17, 2024

   

   koendecock

   on Dec 17, 2024

   Last edited by koendecock

   Author

   More actions

   Hi Colin,

   thank you for this version. I've testen and the compilation failure has more details.

   [INFO] [ERROR] An internal compiler exception occurred
   [INFO] com.google.gwt.dev.jjs.InternalCompilerException: Error constructing Java AST
   [INFO] at com.google.gwt.dev.jjs.impl.GwtAstBuilder.translateException(GwtAstBuilder.java:4190)
   [INFO] at com.google.gwt.dev.jjs.impl.GwtAstBuilder$AstVisitor.visit(GwtAstBuilder.java:1257)
   [INFO] at org.eclipse.jdt.internal.compiler.ast.LambdaExpression.traverse(LambdaExpression.java:787)
   [INFO] at org.eclipse.jdt.internal.compiler.ast.ExplicitConstructorCall.traverse(ExplicitConstructorCall.java:524)
   [INFO] at org.eclipse.jdt.internal.compiler.ast.ConstructorDeclaration.traverse(ConstructorDeclaration.java:704)
   [INFO] at org.eclipse.jdt.internal.compiler.ast.TypeDeclaration.traverse(TypeDeclaration.java:1831)
   [INFO] at org.eclipse.jdt.internal.compiler.ast.TypeDeclaration.traverse(TypeDeclaration.java:1683)
   [INFO] at com.google.gwt.dev.jjs.impl.GwtAstBuilder.processImpl(GwtAstBuilder.java:4126)
   [INFO] at com.google.gwt.dev.jjs.impl.GwtAstBuilder.process(GwtAstBuilder.java:4164)
   [INFO] at com.google.gwt.dev.javac.CompilationStateBuilder$CompileMoreLater$UnitProcessorImpl.process(CompilationStateBuilder.java:128)
   [INFO] at com.google.gwt.dev.javac.JdtCompiler$CompilerImpl.process(JdtCompiler.java:322)
   [INFO] at org.eclipse.jdt.internal.compiler.Compiler.processCompiledUnits(Compiler.java:575)
   [INFO] at org.eclipse.jdt.internal.compiler.Compiler.compile(Compiler.java:475)
   [INFO] at org.eclipse.jdt.internal.compiler.Compiler.compile(Compiler.java:426)
   [INFO] at com.google.gwt.dev.javac.JdtCompiler.doCompile(JdtCompiler.java:1021)
   [INFO] at com.google.gwt.dev.javac.CompilationStateBuilder$CompileMoreLater.compile(CompilationStateBuilder.java:322)
   [INFO] at com.google.gwt.dev.javac.CompilationStateBuilder.doBuildFrom(CompilationStateBuilder.java:532)
   [INFO] at com.google.gwt.dev.javac.CompilationStateBuilder.buildFrom(CompilationStateBuilder.java:464)
   [INFO] at com.google.gwt.dev.cfg.ModuleDef.getCompilationState(ModuleDef.java:426)
   [INFO] at com.google.gwt.dev.Precompile.precompile(Precompile.java:210)
   [INFO] at com.google.gwt.dev.Precompile.precompile(Precompile.java:190)
   [INFO] at com.google.gwt.dev.Precompile.precompile(Precompile.java:131)
   [INFO] at com.google.gwt.dev.Compiler.compile(Compiler.java:192)
   [INFO] at com.google.gwt.dev.Compiler.compile(Compiler.java:143)
   [INFO] at com.google.gwt.dev.Compiler.compile(Compiler.java:132)
   [INFO] at com.google.gwt.dev.Compiler$1.run(Compiler.java:110)
   [INFO] at com.google.gwt.dev.CompileTaskRunner.doRun(CompileTaskRunner.java:55)
   [INFO] at com.google.gwt.dev.CompileTaskRunner.runWithAppropriateLogger(CompileTaskRunner.java:50)
   [INFO] at com.google.gwt.dev.Compiler.main(Compiler.java:113)
   [INFO] Caused by: java.lang.NullPointerException: Cannot read field "declaringScope" because "outerLocalVariable" is null
   [INFO] at org.eclipse.jdt.internal.compiler.lookup.BlockScope.getEmulationPath(BlockScope.java:831)
   [INFO] at com.google.gwt.dev.jjs.impl.GwtAstBuilder$AstVisitor.pushLambdaExpressionLocalsIntoMethodScope(GwtAstBuilder.java:1268)
   [INFO] at com.google.gwt.dev.jjs.impl.GwtAstBuilder$AstVisitor.visit(GwtAstBuilder.java:1254)
   [INFO] ... 27 more
   [INFO] [ERROR] at PdfVersiesView.java(273): (<no type> afdruk) -> {
   [INFO] //...
   [INFO] }
   [INFO] org.eclipse.jdt.internal.compiler.ast.LambdaExpression
   [INFO] ------------------------------------------------------------------------

   It looks like the compiler is complaining about the <no type> of afdruk variable.

   The actual code is
   private class DownloadCell extends ActionImageResourceCell<AfdrukMetaDataResource> { public DownloadCell() { super(afdruk -> { //... }); }

   After changing it to the below code (by not using lambda) resolves the compilation problem .
   private class DownloadCell extends ActionImageResourceCell<AfdrukMetaDataResource> { public DownloadCell() { super(new Delegate<AfdrukMetaDataResource>() { @Override public void execute(AfdrukMetaDataResource afdruk) { //... } }); }

   The parent class ActionImageResourceCell looks like this

   public abstract class ActionImageResourceCell<C> extends ActionCell<C> { public ActionImageResourceCell(ActionCell.Delegate<C> delegate) { super("(ignored)", delegate); }

   Regards, Koen
3. koendecock commented on Dec 18, 2024

   

   koendecock

   on Dec 18, 2024

   Author

   More actions

   I was able to narrow it down. I'll send you 3 test java classes, you can use them to reproduce. The content of the lambda is indeed important. I removed it from the error description I've send you earlier because I thought it was not important. But it is. From the TestComposite.java class I'll send you, the actionBusy = true on line 18 is important. If I remove that line, the compilation is happy. If the line is there, the compilation fails.
   TestFilesToReproduce.zip
4. niloc132 commented on Dec 19, 2024

   

   niloc132

   on Dec 19, 2024

   Member

   More actions

   Thanks, I'm able to reproduce this by adding those to the Hello sample and running ant samples on GWT itself. Both actualOuterLocalVariable and fieldName are null in this case - the variable in question is TestComposite this$0, a reference to the outermost class that contains the lambda (https://stackoverflow.com/a/50093487/860630).

   The <no type> seems to be a red herring, I think the jdt is trying to say that the arg itself didn't have a type defined on it, but the ast nodes recognize correctly that afdruk is a Resource.

   Getting too late for me, but I did quickly dig into "recent" commits made to MethodScope, and found this:
   eclipse-jdt/eclipse.jdt.core@59ad9ed#diff-9d30f7b29768bf2009581713dd97d444f2e734d170b72b8782eed9cfa5879fda

   The test added there looks suspiciously like your test case, but if I'm reading this versioning right, we already have that fix (was added briefly before the jdt-core release that we are using was cut). To me this suggests that we're just using the JDT internals incorrectly (though, perhaps the usage is only incorrect as of this fix being added). I do have a tentative fix that looks too good to be true - I'll run the full CI overnight and if everything seems to pass I'll put up another snapshot, and a PR when I'm more confident of the change.
5. niloc132 commented on Dec 19, 2024

   

   niloc132

   on Dec 19, 2024

   Member

   More actions

   All tests have passed, I've pushed an update to the snapshot version that it appears will fix this. I'll work on better tests and validating that the change makes sense to me, then put up a PR for it.
6. koendecock commented on Dec 20, 2024

   

   koendecock

   on Dec 20, 2024

   Author

   More actions

   Hi Colin, great work. Tnx
7. niloc132 commented on Dec 20, 2024

   

   niloc132

   on Dec 20, 2024

   Member

   More actions

   @koendecock were you able to confirm that this SNAPSHOT resolves the issue for you?
8. koendecock commented on Dec 20, 2024

   

   koendecock

   on Dec 20, 2024

   Author

   More actions

   Hi Colin. I confirm that the patch resolves the issue.
9. niloc132 commented on Jan 2, 2025

   

   niloc132

   on Jan 2, 2025

   Member

   More actions

   A week's worth of notes from puttering with this:

   I've ruled out (at least for the moment) applying the same change in other places, as it introduces other problems where we expect the same synthetic local binding instance that was previously visited - it isn't necessarily required to make this change, but I'm worried about other ways that this bug can happen.

   I am however getting compiler errors with the same minimal test case in GWT 2.11:

        [java]    [ERROR] Errors in 'file:/home/colin/workspace/gwt/samples/hello/src/com/google/gwt/sample/hello/client/TestComposite.java'
        [java]       [ERROR] Line 16: Cannot refer to 'this' nor 'super' while explicitly invoking a constructor
   

   This sort of makes sense to me, if this error was previously (incorrectly) being emitted by JDT, then updating the JDT changed so that we got this current bug instead.

   @koendecock can you confirm that the minimal example you gave succeeds in 2.11 for you?

   ---

   Focusing only on the code that breaks GWT 2.12 with this stack trace, I further reduced the example - a lambda in a non-static inner class in an outer class with a field. The lambda must be declared in a constructor and close over a field in the outermost class, and passed to super()/this():

   public class OuterClass {
     boolean outerField = false;
   
     public static class SuperInnerType {
       public SuperInnerType(Runnable delegate) {
       }
     }
   
     class NonStaticInnerType extends SuperInnerType {
       NonStaticInnerType() {
         // Constructor of a non-static inner class passes a lambda to super() or this(), and
         // that lambda captures a field of the outer class.
         super(() -> {
           outerField = true;
         });
       }
     }
   }

   or

   public class OuterClass {
     boolean outerField = false;
   
     class NonStaticInnerType {
       NonStaticInnerType() {
         // Constructor of a non-static inner class passes a lambda to super() or this(), and
         // that lambda captures a field of the outer class.
         this(() -> {
           outerField = true;
         });
       }
       NonStaticInnerType(Runnable delegate) {
       }
     }
   }

   Note that both of these appear to compile cleanly in GWT 2.11, and fail in GWT 2.12 as reported.

   However, both of these following two tests (written for Java8AstTest) fail in GWT 2.12 (with the NPE), and fail in GWT 2.11 (with the "Cannot refer to 'this' or 'super'" error):

     public void testCompileLambdaOuterFieldCaptureInConstructor() throws UnableToCompleteException {
       addSnippetClassDecl("int y = 22;");
       addSnippetClassDecl("class Foo {" +
           "Foo(Runnable r) {}" +
           "Foo() {" +
               "this(() -> {y = 42;});" +
           "}" +
       "}");
   
       JProgram program = compileSnippet("void", "new Foo();", false);
       assertNotNull(getMethod(program, "lambda$0"));
     }
   
     public void testCompileLambdaOuterFieldCaptureInConstructorSuper() throws UnableToCompleteException {
       addSnippetClassDecl("int y = 22;");
       addSnippetClassDecl("class Foo {" +
           "Foo(Runnable r) {}" +
       "}" +
       "class Bar extends Foo {" +
           "Bar() {" +
               "super(() -> {y = 42;});" +
           "}" +
       "}");
   
       JProgram program = compileSnippet("void", "new Bar();", false);
       assertNotNull(getMethod(program, "lambda$0"));
     }
   

   Why does "OuterClass` succeed in both forms, but your sample and my unit test fail in GWT 2.11? I'm sure I'm just missing something totally unrelated here...
10. niloc132 commented on Jan 2, 2025

    

    niloc132

    on Jan 2, 2025

    Member

    More actions

    Why does "OuterClass` succeed in both forms, but your sample and my unit test fail in GWT 2.11? I'm sure I'm just missing something totally unrelated here...

    Figured it out: the sample I was building wasn't actually instantiating OuterClass, but it was instantiating TestComposite, and -strict was not turned on. Without strict mode, the compiler noticed that these types didn't compile... but ignored the failure since it didn't impact the actual program.

    Similarly, the tests above actually instantiated the test types.

    So now we're at step six: how did this ever work for you with GWT 2.11? Is it possible that this was just dead code that was failing and ignored (note: this is bad for SDM performance, please turn strict on to avoid this), and now that GWT is hitting a bug when compiling it instead of an (incorrect) actual compiler error you saw it?
11. koendecock commented on Jan 6, 2025

    

    koendecock

    on Jan 6, 2025

    Author

    More actions

    Hi Colin, you are right, we are running de gwt compilation with strict mode disabled. And it looks like the code in our project that failed to compile with GWT 2.12 is indeed dead code.
12. niloc132 commented on Jan 6, 2025

    

    niloc132

    on Jan 6, 2025

    Member

    More actions

    Thanks - while technically still a regression, it definitely matters less now since it never worked. A better workaround would be to mark the class itself as @GwtIncompatible or fix your source paths to never include non-compatible files to begin with. Consider using -strict to fix and avoid such errors, this will improve performance of super dev mode by not requiring that it try and fail to recompile this type (and related types) after every single change.
13. niloc132

    added this to the 2.13 milestone on Jan 6, 2025
14. koendecock commented on Jan 7, 2025

    

    koendecock

    on Jan 7, 2025

    Author

    More actions

    Thanks for the hint Colin. We'll enable strict gwt compilation for our projects and try to fix all issues that come across.
15. niloc132

    added 2 commits that reference this issue on Jan 8, 2025

    Exceptions thrown when building GWT's AST should include context

    ...

    0d791d7

    Exceptions thrown when building GWT's AST should include context

    ...

    f39e689
16. niloc132

    closed this as completedin #10078on Feb 5, 2025
17. niloc132

    closed this as completedin f1c4578on Feb 5, 2025
18. niloc132

    added a commit that references this issue on Feb 9, 2025

    Allow lambdas in this()/super() to close over outer types (gwtproject…

    ...

    This issue will close once commit f1c4578 is merged into the 'main' branch.

    f1c4578
19. niloc132

    added a commit that references this issue on Feb 16, 2025

    Exceptions thrown when building GWT's AST should include context (#10080

    ...

    072805e