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

[24] don't request access to enclosing class if it's never used #3194

Closed
stephan-herrmann opened this issue Oct 29, 2024 · 7 comments
Closed
Assignees

Comments

@stephan-herrmann
Copy link
Contributor

Given this example:

	import java.util.function.Supplier;
	@SuppressWarnings("unused")
	class Outer {
		void m() { }
		class Inner {
			Inner() {
				class Foo {
					void g() {
//						m();
					}
				}
				super();
				class Bar {
					static void r() {
						Supplier<Foo> sfoo = Foo::new;
					}
				};
			}
		}
	}

ecj complains against Foo::new:

No enclosing instance of the type Outer is accessible in scope

The message is true, but the ctor of Foo does not need that enclosing instance. This would only be the case when, e.g., the invocation m() is uncommented.

This shows that the mechanics introduced for flexible constructors creates synthetic arguments where none are needed (which then causes an error, when no value can be supplied to that synthetic argument).

@stephan-herrmann
Copy link
Contributor Author

Fixed by #3198

@stephan-herrmann
Copy link
Contributor Author

As of 24-ea+26 javac now answers:

Outer.java:15: error: local class Foo cannot be instantiated from a static context
                                                Supplier<Foo> sfoo = Foo::new;
                                                                     ^

With this shift of focus we are actually challenging a different set of rules, viz. (spec version 20241101):

§ 15.13.1
It is a compile-time error if the method reference expression is of the form ClassType :: [TypeArguments] new and a compile-time error would occur when determining an enclosing instance for ClassType as specified in 15.9.2 (treating the method reference expression as if it were an unqualified class instance creation expression).

So we proceed to 15.9.2 as if Foo::new were new Foo():

§15.9.2:

  • If C is an inner local class, then:
    • If C occurs in a static context, ...
    • Otherwise, if the class instance creation expression occurs in a static context, then a compile-time error occurs.

With our example:

  • Foo is an inner local class
    • Foo is not declared in a static context, so proceed to the next:
    • The instantiation occurs in the static context of method r()

Ergo: we should raise an error

(reminder to self: conversely, also the error on m() when uncommented should be re-checked. It is not raised by javac).

stephan-herrmann added a commit to stephan-herrmann/eclipse.jdt.core that referenced this issue Dec 3, 2024
Fixes after re-opening:
+ essentially revert previous change
+ replace with specific detection wrt local type alloc in static context

Fixes eclipse-jdt#3194
@stephan-herrmann
Copy link
Contributor Author

(reminder to self: conversely, also the error on m() when uncommented should be re-checked. It is not raised by javac).

That one was indeed introduced by #3198 - which is being reverted in #3390

@jarthana
Copy link
Member

jarthana commented Dec 9, 2024

The following case which is accepted in master is now rejected in BETA_JAVA24:

sealed interface A permits X {} 
public final  class X implements A {
	int a = 1;
	class One {
		int b = 2;
		class Two {
			int c = 3;
			class Three {
				int r = a + b + c;
			}
		}
	}
	public static void main(String argv[]) {
		X x = new X();
		One.Two.Three ci = x.new One().new Two().new Three(); // No enclosing instance of type X is accessible. Must qualify the allocation...
		System.out.println(ci.r);
	}
}

@stephan-herrmann let me know if you would prefer a new issue.

@stephan-herrmann
Copy link
Contributor Author

The following case which is accepted in master is now rejected in BETA_JAVA24:

reproduced, thanks for catching

@stephan-herrmann let me know if you would prefer a new issue.

I'm fine with fixing the regression via the same issue.

@stephan-herrmann
Copy link
Contributor Author

Actually, to get back to firm ground, I could use some help to get a green build on any BETA_JAVA24 build, notably #3390 (see also #3388)

stephan-herrmann added a commit to stephan-herrmann/eclipse.jdt.core that referenced this issue Dec 15, 2024
Fixes after re-opening:
+ essentially revert previous change
+ replace with specific detection wrt local type alloc in static context

Fixes eclipse-jdt#3194
@stephan-herrmann
Copy link
Contributor Author

The following case which is accepted in master is now rejected in BETA_JAVA24:

It seems this is already fixed in #3390, just that fix is blocked since two weeks due to broken baseline checks causing the build to fail, see eclipse-platform/eclipse.platform.releng.aggregator#2660

stephan-herrmann added a commit to stephan-herrmann/eclipse.jdt.core that referenced this issue Dec 15, 2024
stephan-herrmann added a commit to stephan-herrmann/eclipse.jdt.core that referenced this issue Dec 17, 2024
Fixes after re-opening:
+ essentially revert previous change
+ replace with specific detection wrt local type alloc in static context

Fixes eclipse-jdt#3194
stephan-herrmann added a commit to stephan-herrmann/eclipse.jdt.core that referenced this issue Dec 17, 2024
stephan-herrmann added a commit that referenced this issue Dec 17, 2024
* [24] don't request access to enclosing class if it's never used

Fixes after re-opening:
+ essentially revert previous change
+ replace with specific detection wrt local type alloc in static context

Fixes #3194

* [24] don't request access to enclosing class if it's never used

+ additional test

Fixes #3194
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

2 participants