Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Adding a custom record constructor prevents successful compilation #10045

Open
andreaskornstaedt opened this issue Nov 14, 2024 · 14 comments
Open

Comments

@andreaskornstaedt
Copy link

andreaskornstaedt commented Nov 14, 2024

**GWT version: 2.12.1
**Browser (with version): any
**Operating System: any


Description

A custom record constructor prevents successful compilation even if never invoked

         Compiling 1 permutation
            Compiling permutation 0...
            [ERROR] An internal compiler exception occurred
com.google.gwt.dev.jjs.InternalCompilerException: Unexpected error during visit.
	at com.google.gwt.dev.jjs.ast.JVisitor.translateException(JVisitor.java:111)
	at com.google.gwt.dev.jjs.ast.JVisitor.accept(JVisitor.java:130)
	at com.google.gwt.dev.jjs.ast.JVisitor.accept(JVisitor.java:122)
	at com.google.gwt.dev.jjs.ast.JTransformer.transform(JTransformer.java:1100)
	at com.google.gwt.dev.jjs.impl.GenerateJavaScriptAST$GenerateJavaScriptTransformer.transform(GenerateJavaScriptAST.java:2659)
	at com.google.gwt.dev.jjs.impl.GenerateJavaScriptAST$GenerateJavaScriptTransformer.transformDeclarationStatement(GenerateJavaScriptAST.java:686)
	at com.google.gwt.dev.jjs.impl.GenerateJavaScriptAST$GenerateJavaScriptTransformer.transformDeclarationStatement(GenerateJavaScriptAST.java:520)
	at com.google.gwt.dev.jjs.ast.JTransformer$JRewriterVisitor.visit(JTransformer.java:725)
	at com.google.gwt.dev.jjs.ast.JDeclarationStatement.traverse(JDeclarationStatement.java:46)
	at com.google.gwt.dev.jjs.ast.JVisitor.accept(JVisitor.java:127)
	at com.google.gwt.dev.jjs.ast.JVisitor.accept(JVisitor.java:122)
	at com.google.gwt.dev.jjs.ast.JTransformer.transform(JTransformer.java:1100)
	at com.google.gwt.dev.jjs.ast.JTransformer.transformIntoExcludingNulls(JTransformer.java:1120)
	at com.google.gwt.dev.jjs.impl.GenerateJavaScriptAST$GenerateJavaScriptTransformer.transformBlock(GenerateJavaScriptAST.java:583)
	at com.google.gwt.dev.jjs.impl.GenerateJavaScriptAST$GenerateJavaScriptTransformer.transformBlock(GenerateJavaScriptAST.java:520)
	at com.google.gwt.dev.jjs.ast.JTransformer$JRewriterVisitor.visit(JTransformer.java:647)
	at com.google.gwt.dev.jjs.ast.JBlock.traverse(JBlock.java:93)
	at com.google.gwt.dev.jjs.ast.JVisitor.accept(JVisitor.java:127)
	at com.google.gwt.dev.jjs.ast.JVisitor.accept(JVisitor.java:122)
	at com.google.gwt.dev.jjs.ast.JTransformer.transform(JTransformer.java:1100)
	at com.google.gwt.dev.jjs.impl.GenerateJavaScriptAST$GenerateJavaScriptTransformer.transform(GenerateJavaScriptAST.java:2672)
	at com.google.gwt.dev.jjs.impl.GenerateJavaScriptAST$GenerateJavaScriptTransformer.transformMethodBody(GenerateJavaScriptAST.java:833)
	at com.google.gwt.dev.jjs.impl.GenerateJavaScriptAST$GenerateJavaScriptTransformer.transformMethodBody(GenerateJavaScriptAST.java:520)
	at com.google.gwt.dev.jjs.ast.JTransformer$JRewriterVisitor.visit(JTransformer.java:854)
	at com.google.gwt.dev.jjs.ast.JMethodBody.traverse(JMethodBody.java:81)
	at com.google.gwt.dev.jjs.ast.JVisitor.accept(JVisitor.java:127)
	at com.google.gwt.dev.jjs.ast.JVisitor.accept(JVisitor.java:122)
	at com.google.gwt.dev.jjs.ast.JTransformer.transform(JTransformer.java:1100)
	at com.google.gwt.dev.jjs.impl.GenerateJavaScriptAST$GenerateJavaScriptTransformer.transformMethod(GenerateJavaScriptAST.java:804)
	at com.google.gwt.dev.jjs.impl.GenerateJavaScriptAST$GenerateJavaScriptTransformer.generatePrototypeDefinitions(GenerateJavaScriptAST.java:2474)
	at com.google.gwt.dev.jjs.impl.GenerateJavaScriptAST$GenerateJavaScriptTransformer.generateTypeSetup(GenerateJavaScriptAST.java:1951)
	at com.google.gwt.dev.jjs.impl.GenerateJavaScriptAST$GenerateJavaScriptTransformer.transformDeclaredType(GenerateJavaScriptAST.java:649)
	at com.google.gwt.dev.jjs.impl.GenerateJavaScriptAST$GenerateJavaScriptTransformer.transformDeclaredType(GenerateJavaScriptAST.java:520)
	at com.google.gwt.dev.jjs.ast.JTransformer.transformClassType(JTransformer.java:95)
	at com.google.gwt.dev.jjs.ast.JTransformer$JRewriterVisitor.visit(JTransformer.java:695)
	at com.google.gwt.dev.jjs.ast.JClassType.traverse(JClassType.java:145)
	at com.google.gwt.dev.jjs.ast.JVisitor.accept(JVisitor.java:127)
	at com.google.gwt.dev.jjs.ast.JVisitor.accept(JVisitor.java:122)
	at com.google.gwt.dev.jjs.ast.JTransformer.transform(JTransformer.java:1100)
	at com.google.gwt.dev.jjs.impl.GenerateJavaScriptAST$GenerateJavaScriptTransformer.transformProgram(GenerateJavaScriptAST.java:1242)
	at com.google.gwt.dev.jjs.impl.GenerateJavaScriptAST$GenerateJavaScriptTransformer.transformProgram(GenerateJavaScriptAST.java:520)
	at com.google.gwt.dev.jjs.ast.JTransformer$JRewriterVisitor.visit(JTransformer.java:943)
	at com.google.gwt.dev.jjs.ast.JProgram.traverse(JProgram.java:1248)
	at com.google.gwt.dev.jjs.ast.JVisitor.accept(JVisitor.java:127)
	at com.google.gwt.dev.jjs.ast.JVisitor.accept(JVisitor.java:122)
	at com.google.gwt.dev.jjs.ast.JTransformer.transform(JTransformer.java:1100)
	at com.google.gwt.dev.jjs.impl.GenerateJavaScriptAST.execImpl(GenerateJavaScriptAST.java:3171)
	at com.google.gwt.dev.jjs.impl.GenerateJavaScriptAST.exec(GenerateJavaScriptAST.java:2944)
	at com.google.gwt.dev.jjs.JavaToJavaScriptCompiler.compilePermutation(JavaToJavaScriptCompiler.java:380)
	at com.google.gwt.dev.jjs.JavaToJavaScriptCompiler.compilePermutation(JavaToJavaScriptCompiler.java:274)
	at com.google.gwt.dev.CompilePerms.compile(CompilePerms.java:198)
	at com.google.gwt.dev.ThreadedPermutationWorkerFactory$ThreadedPermutationWorker.compile(ThreadedPermutationWorkerFactory.java:50)
	at com.google.gwt.dev.PermutationWorkerFactory$Manager$WorkerThread.run(PermutationWorkerFactory.java:74)
	at java.base/java.lang.Thread.run(Thread.java:1583)
Caused by: java.lang.NullPointerException: Cannot invoke "String.isEmpty()" because "actualJsName" is null
	at com.google.gwt.dev.jjs.ast.JMethod.getQualifiedJsName(JMethod.java:172)
	at com.google.gwt.dev.jjs.impl.GenerateJavaScriptAST$GenerateJavaScriptTransformer.transformNewInstance(GenerateJavaScriptAST.java:1002)
	at com.google.gwt.dev.jjs.impl.GenerateJavaScriptAST$GenerateJavaScriptTransformer.transformNewInstance(GenerateJavaScriptAST.java:520)
	at com.google.gwt.dev.jjs.ast.JTransformer$JRewriterVisitor.visit(JTransformer.java:884)
	at com.google.gwt.dev.jjs.ast.JNewInstance.traverse(JNewInstance.java:73)
	at com.google.gwt.dev.jjs.ast.JVisitor.accept(JVisitor.java:127)
	... 52 more
               [ERROR] at Intro.java(36): new Rec("1")
                  com.google.gwt.dev.jjs.ast.JNewInstance
               [ERROR] at Intro.java(36): Rec r = new Rec("1")
                  com.google.gwt.dev.jjs.ast.JDeclarationStatement
               [ERROR] at Intro.java(33): {
  Rec r = new Rec("1");
  Window.alert(r.toString());
}
                  com.google.gwt.dev.jjs.ast.JBlock
               [ERROR] at Intro.java(33): {
  Rec r = new Rec("1");
  Window.alert(r.toString());
}
                  com.google.gwt.dev.jjs.ast.JMethodBody
               [ERROR] at Intro.java(22): co.intro.client.Intro (extends Object implements EntryPoint)
                  com.google.gwt.dev.jjs.ast.JClassType
               [ERROR] at Unknown(0): <JProgram>
                  com.google.gwt.dev.jjs.ast.JProgram
            [ERROR] Unrecoverable exception, shutting down
com.google.gwt.core.ext.UnableToCompleteException: (see previous log entries)
	at com.google.gwt.dev.javac.CompilationProblemReporter.logAndTranslateException(CompilationProblemReporter.java:106)
	at com.google.gwt.dev.jjs.JavaToJavaScriptCompiler.compilePermutation(JavaToJavaScriptCompiler.java:461)
	at com.google.gwt.dev.jjs.JavaToJavaScriptCompiler.compilePermutation(JavaToJavaScriptCompiler.java:274)
	at com.google.gwt.dev.CompilePerms.compile(CompilePerms.java:198)
	at com.google.gwt.dev.ThreadedPermutationWorkerFactory$ThreadedPermutationWorker.compile(ThreadedPermutationWorkerFactory.java:50)
	at com.google.gwt.dev.PermutationWorkerFactory$Manager$WorkerThread.run(PermutationWorkerFactory.java:74)
	at java.base/java.lang.Thread.run(Thread.java:1583)
            [ERROR] Not all permutation were compiled , completed (0/1)
      [WARN] recompile failed
      [WARN] continuing to serve previous version
Steps to reproduce
public record Rec(String s) {
	public Rec(int i) {
		this("" + i);
	}
}

invoked like this

public void onModuleLoad() {
	Rec r = new Rec("1");
	Window.alert(r.toString());
}
Known workarounds

Don't use custom constructors

Links to further discussions
@niloc132
Copy link
Member

niloc132 commented Nov 16, 2024

Workaround:
Declare the default constructor. In this example, that would look like:

public record Rec(String s) {
        public Rec { }
	public Rec(int i) {
		this("" + i);
	}
}

Two stack traces I've identified that can come from this bug - this looks like the one reported:

Caused by: java.lang.NullPointerException: Cannot invoke "String.isEmpty()" because "actualJsName" is null
	at com.google.gwt.dev.jjs.ast.JMethod.getQualifiedJsName(JMethod.java:172)
	at com.google.gwt.dev.jjs.impl.GenerateJavaScriptAST$GenerateJavaScriptTransformer.transformNewInstance(GenerateJavaScriptAST.java:1002)
	at com.google.gwt.dev.jjs.impl.GenerateJavaScriptAST$GenerateJavaScriptTransformer.transformNewInstance(GenerateJavaScriptAST.java:520)
	at com.google.gwt.dev.jjs.ast.JTransformer$JRewriterVisitor.visit(JTransformer.java:884)
	at com.google.gwt.dev.jjs.ast.JNewInstance.traverse(JNewInstance.java:73)
	at com.google.gwt.dev.jjs.ast.JVisitor.accept(JVisitor.java:127)
	... 52 more

And another that is similar, but not quite the same:

Caused by: java.lang.NullPointerException: Cannot invoke "String.isEmpty()" because "actualJsName" is null
	at com.google.gwt.dev.jjs.ast.JMethod.getQualifiedJsName(JMethod.java:172)
	at com.google.gwt.dev.jjs.impl.GenerateJavaScriptAST$GenerateJavaScriptTransformer.dispatchToSuper(GenerateJavaScriptAST.java:920)
	at com.google.gwt.dev.jjs.impl.GenerateJavaScriptAST$GenerateJavaScriptTransformer.transformMethodCall(GenerateJavaScriptAST.java:898)
	at com.google.gwt.dev.jjs.impl.GenerateJavaScriptAST$GenerateJavaScriptTransformer.transformMethodCall(GenerateJavaScriptAST.java:520)
	at com.google.gwt.dev.jjs.ast.JTransformer$JRewriterVisitor.visit(JTransformer.java:860)
	at com.google.gwt.dev.jjs.ast.JMethodCall.traverse(JMethodCall.java:265)
	at com.google.gwt.dev.jjs.ast.JVisitor.accept(JVisitor.java:127)
	... 58 more

The bug is that when being called in this way, the "super" constructor is incorrectly identified as being a native method as it has no body. I'm still working out why it has no body - it appears that either JDT doesn't emit a body for the canonical record constructor if there is a non-default constructor declared, or GWT just doesn't read it properly - but if the canonical constructor is declared or no constructors are declared at all, it will correctly have the field assignments as expected.

@niloc132
Copy link
Member

This is an upstream JDT bug, already fixed there: eclipse-jdt/eclipse.jdt.core#365. It appears that updating to 3.33 may resolve this. That version is also compiled to work with Java 11 so we should be safe to update, but it might be risky to do so on a point release like 2.12.2.

We should be able to work around it in a way that still works when JDT is later updated, by detecting if the body is missing from the canonical constructor, and if so, generate the field assignments. There is a caveat here that this must end up in the GwtAstBuilder rather than in ImplementRecordComponents - since the field assignments never were generated, the first pruneDeadFieldsAndMethods pass that runs as part of UnifyAst.exec ends up removing those fields outright, so we can't generate the correct constructor later.

@andreaskornstaedt
Copy link
Author

Workaround: Declare the default constructor. In this example, that would look like:

public record Rec(String s) {
        public Rec { }
	public Rec(int i) {
		this("" + i);
	}
}

Very helpful 👍

@vegegoku
Copy link
Contributor

vegegoku commented Dec 19, 2024

Unfortunately, JDT 3.33.0 does not fix this, it is 3.34.0 that fixes it, but 3.34.0 will require that we drop Java 11 support and build GWT with Java 17 .

I have updated GWT JDT and a made a local build with both versions of JDT to test it.

@FrankHossfeld
Copy link
Member

I assume, we will see this happen more often in the future. Many components start required at least Java 17. May be it is time to start a discussion if GWT still support Java 11 or not and request at least Java 17.

@niloc132
Copy link
Member

Agreed - let's raise this in the new year on the mailing list. We would be in good company dropping support for Java 11, but at the same time I'm guessing there are more than a few projects which successfully updated from Java 8 and are not ready to move past 11 for a little while. Reviewing some of the "state of Java ecosystem 2024" surveys, the percent of projects using Java 11 is roughly the same as last year, though given the trends with 8/17/21 I am guessing that this indicates mostly the portion of Java 8 projects that updated but couldn't yet move to 17 or 21.

The good news there is that we haven't heard any pushback at all on dropping Java 8, so we likely aren't in a position to backport anything to 2.11 - but if we make 2.12 the end of Java 11 support, we may end up with a string of backport releases there.

@niloc132
Copy link
Member

@vegegoku can you comment if any other changes were required to update to 3.34 in GWT? If not, and we resolve #10026, then projects can update JDT on their own if they support Java 17?

@vegegoku
Copy link
Contributor

vegegoku commented Dec 22, 2024

@vegegoku can you comment if any other changes were required to update to 3.34 in GWT? If not, and we resolve #10026, then projects can update JDT on their own if they support Java 17?

From how it looks it requires GWT itself to be built with 3.34.0 at least, which means GWT itself will be built with Java 17, here is a summary of my testing using unpacked jars as suggested in #10026 :

  1. Using JDT 3.33.0 and building GWT with Java 11 -> but not fixed. ❌
  2. Using JDT 3.33.0 and building GWT with Java 17 -> Bug not fixed. ❌
  3. Using JDT 3.34.0 GWT must be built with Java 17 -> Bug is fixed. ✔️

@vegegoku
Copy link
Contributor

If we would release gwt-dev without the JDT, then we can build gwt using java 11 and JDT 3.33.0 then in the user application they can add a dependency on jdt 3.34.0 and build with java 17 and the issue should be fixed.

@FrankHossfeld
Copy link
Member

This is a possible solution. However, one that requires a configuration for GWT to run flawlessly. Looking back, this would be the first time in GWT's history that a user has to make a configuration before a build works. I think we should, as @niloc132 already suggested, start a survey and make a decision depending on the outcome. Personally, I would be in favor of upgrading to Java 17.

@vegegoku
Copy link
Contributor

Part of my experiment was related to some work being done to move gwt build from ant to maven, and this was a confirmation that this kind of limitation can be avoided when we switch and provides a work around so people wont be blocked by this until the next release or so.

@FrankHossfeld
Copy link
Member

That's what I understood. I just wanted to point out that it will be the first time in the history of GWT that a dependency needs to be added for a successful build and the questions that come with it.

And, you also have to ask the question, what effects does this have on third-party libs.

However, it is good to know that there is a workaround.

@niloc132
Copy link
Member

I think you still misunderstand slightly @FrankHossfeld - the linked #10026 issue would not require any other dependencies added except to resolve the current issue. For any user not using records, or using records with the workaround provided here, it would be sufficient to ignore this custom setup - and we will not close this issue at all until HEAD-SNAPSHOT without extra dependencies has the bug fixed.

Instead, when the JDT is removed from the maven copy of gwt-dev.jar (like asm, htmlunit, and a few others), the gwt-dev pom.xml will include a dependency to it, so user projects will transitively pick it up. Projects not using artifacts from maven central will still include the jdt classes in the jar, so they will remain unaffected (and cannot apply this workaround).

@FrankHossfeld
Copy link
Member

Yeap, you are right. Thought anyone has to do it with the upcoming release.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants