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

Spoon representation does not model the implicit String and int parameters to enum constructors #4758

Closed
calebvoss opened this issue Jun 23, 2022 · 6 comments · Fixed by #4769

Comments

@calebvoss
Copy link

The constructor for java.lang.Enum takes two arguments, String and int. Accordingly, the default constructor for any enum type, unlike for ordinary classes, implicitly accepts these two arguments and forwards them to the super constructor. User-defined enum constructors can have additional explicit arguments; the signature of a user-defined enum constructor has the implicit String and int parameters prepended to it.

So, for example, the source code

enum MyEnum {}

is converted by javac to something that essentially looks like

class MyEnum extends java.lang.Enum {
  MyEnum (String s, int i) {
    super(s, i);
  }
}

Also, the source code

enum MyEnum2 {
  MyEnum2 (double x) {}
}

is converted by javac to something that essentially looks like

class MyEnum2 extends java.lang.Enum {
  MyEnum2 (String s, int i, double x) {
    super(s, i);
  }
}

One can view the disassembly with javap -c -p -s to see this.

The Spoon representation currently does not model the implicit String and int parameters, omitting them from an enum's constructor parameters and from the call to the super constructor. This is incorrect, because java.lang.Enum does not have a zero-argument constructor.

@MartinWitt MartinWitt added bug and removed bug labels Jun 23, 2022
@SirYwell
Copy link
Collaborator

It's a bit more complicated: The two parameters inserted by javac are synthetic, not implicit. They are not required to exist and other compilers can choose to do something else. The JLS § 8.9.2 mentions:

In practice, a compiler is likely to mirror the Enum class by declaring String and int parameters in the default constructor of an enum class. However, these parameters are not specified as "implicitly declared" because different compilers do not need to agree on the form of the default constructor. Only the compiler of an enum declaration knows how to instantiate the enum constants; other compilers can simply rely on the implicitly declared public static fields of the enum class (§8.9.3) without regard for how those fields were initialized.

So, as synthetic constructs are compiler-dependend, it's basically impossible for spoon to model them correctly.

However, I just saw that the implicit methods valueOf(String name) and values() are not modelled by spoon either, which it probably should...

@calebvoss
Copy link
Author

I see, that does make it more complicated.

But the semantics of the enum, in regard to the String names and int ordinals, is uniquely defined, even if the implementation is not uniquely defined. Am I right about this? I believe the compiler does not have the freedom to choose arbitrary names or ordinals for the enum values. They are well-defined and uniquely defined. Isn't it then a correct modeling of the program semantics for spoon to, as a compiler, choose any legal implementation with which to produce the required (and explicit) arguments to the java.lang.Enum constructor?

@I-Al-Istannen
Copy link
Collaborator

Hey,

my main question is: What problem are you trying to solve that you currently can not due to this?

This is incorrect

I am not sure I agree, as your source code clearly shows exactly this to be correct!
I don't believe that spoon should model this. We currently explicitly exclude the implicit this parameter for inner classes, we do not concern ourselves with synthetic bridge methods or bootstrap handles and so on, as they are mostly implementation details and not relevant for the language.

@calebvoss
Copy link
Author

The item that is incorrect is one very specific thing: In the constructor of an enum, spoon generates a call to super(), referencing the sole constructor of java.lang.Enum. But this constructor has two explicit arguments (again, these are not synthetic).

The source code examples I gave earlier are not a contradiction here, since the source program does not explicitly invoke the super constructor. Indeed, it is forbidden for the source program to have a super constructor call in the constructor of an enum, (JLS 8.9.2), presumably exactly for the reason that it is the compiler, not the user, who must supply the arguments to the super constructor.

What problem are you trying to solve that you currently can not due to this?

The upshot is that spoon generates a representation that fails to type-check, which is problematic for my use case. I can certainly come up with a workaround that rewrites the representation into something that type-checks, but I still felt it appropriate to file a bug report since the tool currently emits an invalid function call. Perhaps it is best not to emit the call at all, in the spirit of not modelling things that are implementation details?

@I-Al-Istannen
Copy link
Collaborator

Perhaps it is best not to emit the call at all, in the spirit of not modelling things that are implementation details?

That sounds like the best solution to me, though it is a bit weird. Less so than the alternatives though IHMO.
I will have a shot at implementing it in the next few days, if you or somebody else doesn't beat me to it.

The upshot is that spoon generates a representation that fails to type-check, which is problematic for my use case.

Is this actually observable (e.g. printed) or does your type-checker consider the implicit (hopefully marked as such) super invocation and fails while doing so?

@I-Al-Istannen
Copy link
Collaborator

I have had a look at the issue and the source for the call is JDT itself. It seems like we build the AST before JDT has lowered the constructor far enough to add the String and int parameters to it.

I've linked a PR that just removes the super call in such a case. Not ideal, but I do agree that the current version is unacceptable and this sounded better than manually adding compiler internals back into the model.

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

Successfully merging a pull request may close this issue.

4 participants