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

Eclipse incorrectly requires catch for nested sneaky throws; OpenJDK compiles with no problem #1162

Closed
garretwilson opened this issue Jun 19, 2023 · 23 comments · Fixed by #1166
Assignees
Milestone

Comments

@garretwilson
Copy link

In Eclipse EE 2023-06 with Java 17 on Windows 10, my project is using the sneaky throws technique via the Faux Pas library. This technique is not new, and many projects have been using it with no problems, as have I for many years.

I'm using a method that looks like this:

public void showEclipseProblem() throws IOException {
  final DocumentBuilder documentBuilder = null; //TODO
  try (final Stream<Path> barDirs = Files.list(Paths.get("foo"))) {
    barDirs.forEach(FauxPas.throwingConsumer(barDir -> {
      try (final Stream<Path> files = Files.list(barDir)) {
        files.forEach(FauxPas.throwingConsumer(file -> {
          final Document document;
          try (final InputStream inputStream = new BufferedInputStream(newInputStream(file))) {
//TODO uncomment this line to see the problem:             document = documentBuilder.parse(inputStream);
          }
        }));
      }
    }));
  }
}

This code compiles fine on Eclipse EE 2023-06 and on OpenJDK Runtime Environment Temurin-17.0.4+8 (build 17.0.4+8).

When I uncomment the line document = documentBuilder.parse(inputStream), OpenJDK continues to compile the code with no problems. However Eclipse suddenly underlines Files.list(barDir) in red, indicating the following error:

Unhandled exception type IOException

This error makes no logical sense, because Files.list(barDir) is inside FauxPas.throwingConsumer(…), which should allow the IOException to be thrown with no declaration or catch. In fact to show how senseless this is, I can actually change the code to throw an exception manually, and Eclipse (correctly) finds no problem with it:

public void showEclipseProblem() throws IOException {
  final DocumentBuilder documentBuilder = null; //TODO
  try (final Stream<Path> barDirs = Files.list(Paths.get("foo"))) {
    barDirs.forEach(throwingConsumer(barDir -> {
      try (final Stream<Path> files = Files.list(barDir)) {
        throw new IOException("Eclipse correctly doesn't think this needs to be caught.");
      }
    }));
  }
}

Furthermore it's odd that adding a completely unrelated line document = documentBuilder.parse(inputStream) would cause this error to appear on a completely other line altogether.

It would appear Eclipse is just getting its wires crossed somewhere, as they say.

@garretwilson garretwilson changed the title Eclipse requires exception declaration for nested sneaky throws; OpenJDK compiles with no problem Eclipse incorrectly requires catch for nested sneaky throws; OpenJDK compiles with no problem Jun 19, 2023
@srikanth-sankaran
Copy link
Contributor

If you can provide a full standalone test case I can take a look - I tried stubbing out various symbols and importing a bunch with no immediate success. I can invest more time but how can I be sure I will be fixing the real problem and not some concocted variant ?

@garretwilson
Copy link
Author

garretwilson commented Jun 20, 2023

I don't understand what you meant about stubbing out various symbols. Did you copy and paste the sample code in Eclipse EE 2023-06 using OpenJDK 17 on Windows 10?

If you can provide a full standalone test case I can take a look …

I spent a lot of time narrowing this down to provide you a small piece of actual code with no additional dependencies. I don't know how it could be much more standalone. Do you have doubts about some of the imports? If so let me know and I can help you out.

If you have a question about my setup, let me know and I'll clear it up. If you reproduce my setup but don't see the issue, then we'll go from there.

@srikanth-sankaran
Copy link
Contributor

I don't understand what you meant about stubbing out various symbols. Did you copy and paste the sample code in Eclipse EE 2023-06 using OpenJDK 17 on Windows 10?

As a JDT committer, I work with/on Eclipse SDK builds not EE or any other variant. So it would help to have a test case that reproduces the problem on plain vanilla Eclipse.

If you can provide a full standalone test case I can take a look …

I spent a lot of time narrowing this down to provide you a small piece of actual code with no additional dependencies. I don't know how it could be much more standalone. Do you have doubts about some of the imports? If so let me know and I can help you out.

Thanks. So am I to understand EE version would allow me to to just paste that method without a class wrapping it and without imports ? That is interesting -

If you have a question about my setup, let me know and I'll clear it up. If you reproduce my setup but don't see the issue, then we'll go from there.

I offer to investigate if I can see the issue in my setup - which is plain vanilla eclipse SDK of 4.28 version. As I see, a minimal example in this approach may require you to provide skeleton classes with just the minimal involved symbols (such as FauxPas - make that a class in the test case itself - rather than come from a library on build path) - this is what I meant by stubbing out various symbols.

@garretwilson
Copy link
Author

So am I to understand EE version would allow me to to just paste that method without a class wrapping it and without imports ?

I'm honestly having trouble determining if that was sarcasm, a rhetorical question, or an honest doubt about how the EE version of Eclipse works.

Let me just say that this is a very technical, intricate bug that will involve a deep understanding of type theory, generics theory, and as well as likely years of experience with the Eclipse implementation. (I say this from experience from other bugs I've filed over the years such as Bug 441905 and a host of others.) I have a hunch, based upon experience with these types of bugs, that I myself would have trouble finding the cause of this bug without deep experience in the Eclipse Java compiler implementation.

If someone has trouble figuring out which imports to use for Java NIO Path, then honestly I doubt that person will make much progress on resolving this bug, even if I waste a day providing a Git repository with a full Maven project to import.

And now that I review Bug 441905, @srikanth-sankaran it looks like you were part of that discussion, so I conclude 1) you are probably one of the few people who actually do have the expertise to investigate this (even more than I do), and 2) your question about Eclipse EE was sarcastic or at least rhetorical, so you know the answer already.

Like I said if you reproduce my setup and don't see the bug, or if you have an honest question about which imports are needed, let me know and I'll be happy to answer. Otherwise this is all I have time to provide at the moment.

@srikanth-sankaran
Copy link
Contributor

So am I to understand EE version would allow me to to just paste that method without a class wrapping it and without imports ?

I'm honestly having trouble determining if that was sarcasm, a rhetorical question, or an honest doubt about how the EE version of Eclipse works.

Truth can be stranger than fiction. It was a straightforward question. I have never used EE. Various frameworks do their own magic. Your question reproduced verbatim here: Did you copy and paste the sample code in Eclipse EE 2023-06 using OpenJDK 17 on Windows 10? suggests a wrapping class or imports are not needed. I took you at face value.

If someone has trouble figuring out which imports to use for Java NIO Path, then honestly I doubt that person will make much progress on resolving this bug, even if I waste a day providing a Git repository with a full Maven project to import.

It was not the import for NIO Path. It was various things one after the other - Document comes with two import choices. May be to you it is obvious which one it is - to me it is a guess. I invite you to start with the method you have in Eclipse SDK and fill in the blanks to see for yourself.

BTW, my idea of a standalone test would not involve maven. I don't know the first thing about maven. Never used it. A good reproducer for me is a snippet inline in the report or an eclipse project that I can import and see the problem without having to do anything else.

And now that I review Bug 441905, @srikanth-sankaran it looks like you were part of that discussion, so I conclude 1) you are probably one of the few people who actually do have the expertise to investigate this (even more than I do), and 2) your question about Eclipse EE was sarcastic or at least rhetorical, so you know the answer already.

Sarcasm is unprofessional. I wasn't being sarcastic.

Perhaps your bewilderment originates from a disbelief that folks may work on JDT but not know the first thing about EE. That demographic exists and is populated by at at least one individual.

Here are the issues closed in JDT by that individual:

https://bugs.eclipse.org/bugs/buglist.cgi?bug_status=RESOLVED&bug_status=VERIFIED&bug_status=CLOSED&classification=Eclipse%20Project&component=APT&component=Core&component=Debug&component=Doc&component=Text&component=UI&email1=srikanth&emailassigned_to1=1&emailtype1=substring&list_id=21356296&product=JDT&query_format=advanced

You have my offer to investigate if I am able to reproduce in vanilla Eclipse SDK.

If that is not possible, this will eventually get attention from someone more knowledgeable than me.

@garretwilson
Copy link
Author

garretwilson commented Jun 20, 2023

Truth can be stranger than fiction. It was a straightforward question. I have never used EE.

In that case I apologize for interpreting it incorrectly. I thought it was a jab at me for not providing imports. 😬 My mistake.

No, as far as I understand, Eclipse EE just adds extra tools/extensions/plugins for working with EE (I guess that's Jakarta) stuff like servlets and (supposedly) helpful XML editors and the like. As far as I know Eclipse EE should have zero bearing on this bug, but I wanted to mention my specific configuration because I know that's helpful. Who know—maybe one of its included extensions is causing some weird interaction (but I highly doubt it).

It was various things one after the other - Document comes with two import choices.

Like I said I'll be happy to address any doubts you have about the imports.

In this case it was org.w3c.dom.Document. I probably didn't think to mention this because of my decades of working closely with the W3C DOM. Although to be fair there is only one DocumentBuilder in the JDK and its DocumentBuilder.parse(…) produces a org.w3c.dom.Document. It only takes a couple of seconds to hit Ctrl+Space—certainly less time that it takes me to go put together a cloneable project.

You have my offer to investigate if I am able to reproduce in vanilla Eclipse SDK.

My hunch is that the difference doesn't have anything to do with anything in Eclipse EE and this is only a compiler issue, but again I tried to specify my full environment exactly, indicating anything that you wouldn't know or couldn't easily find out.

I hope you can reproduce it in the vanilla Eclipse SDK. Good luck!

@garretwilson
Copy link
Author

garretwilson commented Jun 20, 2023

One other thing that might be helpful to know (I'm just thinking of anything that might be useful) is that my Maven POM is using <maven.compiler.release>17</maven.compiler.release>. (If you don't know much about Maven then this may not mean to much to you. I highly recommend you at least become acquainted with Maven/Gradle builds; it's hard to understand how one could get by in 2023 without at least a working knowledge.)

I don't think this has do do with Maven, but there are a few things that might influence m2e and therefore the compiler configuration. The Maven Compiler Plugin translates <maven.compiler.release>17</maven.compiler.release> into a compiler flag, but here it gets tricky, because there are different ways to set the "Java version"; see https://stackoverflow.com/a/65655542 and https://stackoverflow.com/q/64868952 for some of the sordid details.

I don't know how how m2e uses this to set the Java version in Eclipse, and unfortunately there is a bug in m2e (that unfortunately even made it into Eclipse 2023-06) that do the wrong things with facets on projects based upon maven.compiler.release. See eclipse-m2e/m2e-core#1414. That's probably not related to this bug. But since this bug has to do with the compiler, who knows? I'm just trying to provide all information I can. I don't know how setting the compiler version in Eclipse without using m2e compares to setting maven.compiler.release in Maven.

@srikanth-sankaran
Copy link
Contributor

srikanth-sankaran commented Jun 20, 2023

As an academic exercise, I will enumerate the steps I am taking:

  • Copy + paste the snippet from top into Eclipse SDK
  • Create a class X11 to wrap the method showEclipseProblem
  • Use quick fix to import java.io.IOException;
  • Use quick fix to import javax.xml.parsers.DocumentBuilder;
  • Use quick fix to import java.util.stream.Stream;
  • Use quick fix to import java.nio.file.Path;
  • Use quick fix to import java.nio.file.Paths;
  • Use quick fix to import java.nio.file.Files;

At this point I am left with one error:

FauxPas cannot be resolved

I want to include a stub of this class, google search leads me to this link:
https://github.com/zalando/faux-pas/blob/ba39f1d71788020fb14264ee683ebe758344ad03/src/main/java/org/zalando/fauxpas/FauxPas.java#L17

from here I concoct the following and include it in the test case we are building:

@FunctionalInterface
interface ThrowingConsumer<T, X extends Throwable> extends Consumer<T> {

    void tryAccept(T t) throws X;

    @Override
    @SneakyThrows
    default void accept(final T t) {
        tryAccept(t);
    }

}
final class FauxPas {
	public static <T, X extends Throwable> ThrowingConsumer<T, X> throwingConsumer(
            final ThrowingConsumer<T, X> consumer) {
        return consumer;
    }
}

Continuing along

  • Use quick fix to import java.util.function.Consumer;
  • import org.w3c.dom.Document;

Now I am left with 4 errors.

Description Resource Path Location Type
BufferedInputStream cannot be resolved to a type X.java /pasted_code/src line 21 Java Problem
InputStream cannot be resolved to a type X.java /pasted_code/src line 21 Java Problem
SneakyThrows cannot be resolved to a type X.java /pasted_code/src line 37 Java Problem
The method newInputStream(Path) is undefined for the type X X.java /pasted_code/src line 21 Java Problem

Here. I don't get a quick fix offered for InputStream or BufferedInputStream -

I manually add:

import java.io.InputStream;
import java.io.BufferedInputStream;

I am left with two errors now:

Description	Resource	Path	Location	Type
SneakyThrows cannot be resolved to a type	X.java	/_pasted_code_/src	line 40	Java Problem
The method newInputStream(Path) is undefined for the type X	X.java	/_pasted_code_/src	line 24	Java Problem

Googling for SneakyThrows.java I pick up this snippet to add to the test case:

@Target({ElementType.METHOD, ElementType.CONSTRUCTOR})
@Retention(RetentionPolicy.SOURCE)
@interface SneakyThrows {
	/** @return The exception type(s) you want to sneakily throw onward. */
	Class<? extends Throwable>[] value() default java.lang.Throwable.class;
	
	//The fully qualified name is used for java.lang.Throwable in the parameter only. This works around a bug in javac:
	//   presence of an annotation processor throws off the type resolver for some reason.
}

This creates a few more compile errors:

  • Add import java.lang.annotation.*;

to get rid of a bunch of them and I am left with:

Description	Resource	Path	Location	Type
The method newInputStream(Path) is undefined for the type X	X.java	/_pasted_code_/src	line 26	Java Problem
Unhandled exception type X	X.java	/_pasted_code_/src	line 44	Java Problem

It was at this point that I stopped chasing the goose and asked for a standalone test.

But now continuing further:

  • I guess a static import of import static java.nio.file.Files.newInputStream; is missing, add it.

I am left with this snippet and one error:

import java.io.IOException;
import java.nio.file.Files;
import java.nio.file.Path;
import java.nio.file.Paths;
import java.util.function.Consumer;
import java.util.stream.Stream;
import static java.nio.file.Files.newInputStream;

import javax.xml.parsers.DocumentBuilder;

import org.w3c.dom.Document;

import java.io.InputStream;
import java.lang.annotation.*;

import java.io.BufferedInputStream;


public class X11 {
public void showEclipseProblem() throws IOException {
  final DocumentBuilder documentBuilder = null; //TODO
  try (final Stream<Path> barDirs = Files.list(Paths.get("foo"))) {
    barDirs.forEach(FauxPas.throwingConsumer(barDir -> {
      try (final Stream<Path> files = Files.list(barDir)) {
        files.forEach(FauxPas.throwingConsumer(file -> {
          final Document document;
          try (final InputStream inputStream = new BufferedInputStream(newInputStream(file))) {
//TODO uncomment this line to see the problem:             document = documentBuilder.parse(inputStream);
          }
        }));
      }
    }));
  }
}
}

@FunctionalInterface
interface ThrowingConsumer<T, X extends Throwable> extends Consumer<T> {

    void tryAccept(T t) throws X;

    @Override
    @SneakyThrows
    default void accept(final T t) {
        tryAccept(t);
    }

}
final class FauxPas {
	public static <T, X extends Throwable> ThrowingConsumer<T, X> throwingConsumer(
            final ThrowingConsumer<T, X> consumer) {
        return consumer;
    }
}

@Target({ElementType.METHOD, ElementType.CONSTRUCTOR})
@Retention(RetentionPolicy.SOURCE)
@interface SneakyThrows {
	/** @return The exception type(s) you want to sneakily throw onward. */
	Class<? extends Throwable>[] value() default java.lang.Throwable.class;
	
	//The fully qualified name is used for java.lang.Throwable in the parameter only. This works around a bug in javac:
	//   presence of an annotation processor throws off the type resolver for some reason.
}

Error from ECJ being:

Description	Resource	Path	Location	Type
Unhandled exception type X	X11.java	/_pasted_code_/src	line 45	Java Problem

Now javac is also complaining about this snippet:

C:\tmp>javac -g X11.java
X11.java:45: error: unreported exception X; must be caught or declared to be thrown
        tryAccept(t);
                 ^
  where X is a type-variable:
    X extends Throwable declared in interface ThrowingConsumer
1 error

Now uncommenting the line you want me to uncomment, I get this final snippet:

import java.io.IOException;
import java.nio.file.Files;
import java.nio.file.Path;
import java.nio.file.Paths;
import java.util.function.Consumer;
import java.util.stream.Stream;
import static java.nio.file.Files.newInputStream;

import javax.xml.parsers.DocumentBuilder;

import org.w3c.dom.Document;

import java.io.InputStream;
import java.lang.annotation.*;

import java.io.BufferedInputStream;


public class X11 {
public void showEclipseProblem() throws IOException {
  final DocumentBuilder documentBuilder = null; //TODO
  try (final Stream<Path> barDirs = Files.list(Paths.get("foo"))) {
    barDirs.forEach(FauxPas.throwingConsumer(barDir -> {
      try (final Stream<Path> files = Files.list(barDir)) {
        files.forEach(FauxPas.throwingConsumer(file -> {
          final Document document;
          try (final InputStream inputStream = new BufferedInputStream(newInputStream(file))) {
           document = documentBuilder.parse(inputStream);
          }
        }));
      }
    }));
  }
}
}

@FunctionalInterface
interface ThrowingConsumer<T, X extends Throwable> extends Consumer<T> {

    void tryAccept(T t) throws X;

    @Override
    @SneakyThrows
    default void accept(final T t) {
        tryAccept(t);
    }

}
final class FauxPas {
	public static <T, X extends Throwable> ThrowingConsumer<T, X> throwingConsumer(
            final ThrowingConsumer<T, X> consumer) {
        return consumer;
    }
}

@Target({ElementType.METHOD, ElementType.CONSTRUCTOR})
@Retention(RetentionPolicy.SOURCE)
@interface SneakyThrows {
	/** @return The exception type(s) you want to sneakily throw onward. */
	Class<? extends Throwable>[] value() default java.lang.Throwable.class;
	
	//The fully qualified name is used for java.lang.Throwable in the parameter only. This works around a bug in javac:
	//   presence of an annotation processor throws off the type resolver for some reason.
}

which produces two errors in ECJ and one with javac - including the puzzling one you call out in Files.list(barDir)

Now would you say this captures the scenario properly ? Is the fact that ECJ produces 2 errors vs javac producing one the problem ?

My point simply is in taking these various steps, I am being put in the position of guessing, whereas if you do these as the reporter you are in a better position to eliminate guesswork.

@srikanth-sankaran
Copy link
Contributor

it's hard to understand how one could get by in 2023 without at least a working knowledge.)

Swimmingly! thank you :) That is good natured humor not trying to be snide or sarcastic :)

@garretwilson
Copy link
Author

Swimmingly! thank you :) That is good natured humor not trying to be snide or sarcastic :)

I hope you took it as such! It was an honest recommendation. On a related note, I recommend the Manning book The Well-Grounded Java Developer, Second Edition which goes over several things (including Maven) that a modern developer should know about. It's what I'm using as my light reading nowadays (as opposed to the book on Rust).

@srikanth-sankaran
Copy link
Contributor

Now would you say this captures the scenario properly ? Is the fact that ECJ produces 2 errors vs javac producing one the problem ?

Please confirm. Is the last snippet representative of the problem ? Is your complaint about the extra error from ECJ ? TIA

@garretwilson
Copy link
Author

garretwilson commented Jun 20, 2023

My point simply is in taking these various steps, I am being put in the position of guessing …

No, you seem to have misinterpreted one of the fundamental pieces of this bug.

FauxPas cannot be resolved

I want to include a stub of this class

No. You can't stub it out. It's part and parcel to the entire behavior here.

I gave a link to the Faux Pas library. If you were working with Maven you would just include its coordinates. If you don't work with Maven … I suppose there is a JAR you can download, but … why? Just reference org.zalando:faux-pas:x.x.x in your Maven or Gradle configuration.

Have you worked with "sneaky throws" before? If not, then no wonder you find this whole thing confusing. I referenced the Lombok SneakyThrows article as a quick summary. But if you've never heard of it, maybe check out the Baeldung article. The Faux Pas project documentation is also reasonably good.

The point here is that this entire thing about Faux Pas is the core of the issue. Stubbing it out will miss the bug altogether, because the whole point is that sneaky throws hides exceptions and let them sneak through things like lambdas in streams without needing them to be declared.

I confess I only read the first and last of your large comment, because I wanted to clarify that point. I'll go back and read the rest now.

@garretwilson
Copy link
Author

garretwilson commented Jun 20, 2023

OK I read the long comment on reproducing this, but I only skimmed it, because I think it's what I realized in my last comment: you're trying to stub the Faux Pas library, which is the core of this whole bug with something called "sneaky throw" which I don't think you're familiar with.

I linked to the Faux Pas library in the description, as well as the Lombok overview of the technique.

You can say "but sneaky throw is a compiler trick", and it definitely is. But based upon the Java compiler specification, the trick should work, and it does work fine in OpenJDK, and in fact it works just fine 95% of the time in Eclipse too, except in this case Eclipse gets confused and starts complaining about a missing catch for an exception type, based upon some arbitrary line somewhere else, and when it (correctly) doesn't complain even if I add an literal throw new IOException(…) around the same location. But now I'm repeating the bug description. 😉

@srikanth-sankaran
Copy link
Contributor

My gut feel is that we have enough in this standalone self contained test case to investigate. That it produces an error extra with ECJ - which is the puzzling error you call out at the top - tells me we have captured the essence of the defect. I may actually need the FauxPas library to run the program - but to get ECJ behavior to match javac behavior may be enough.

@garretwilson
Copy link
Author

Interesting. You're saying that the error may be completely unrelated to the sneaky throw with Faux Pas? I don't know, because if my code has two errors I never think to compare it with Open JDK to see if the Open JDK has one fewer error. I just fix both errors. 😅 That's why I just assumed this was related to sneaky throws—the reason this came up is that because of FauxPas, I should have no errors (as the Open JDK does), but in Eclipse I get one error. (That's also why it confused me when you were talking about two versus one error.)

If you can reproduce it without Faux Pas, even better.

But if you want to produce is faster and not be distracted with additional errors, you might consider adding Faux Pas. Whatever works for you. Good luck.

@srikanth-sankaran
Copy link
Contributor

Interesting. You're saying that the error may be completely unrelated to the sneaky throw with Faux Pas? I don't know, because if my code has two errors I never think to compare it with Open JDK to see if the Open JDK has one fewer error.

When you are in the business of writing compilers, you compare and compare often with the "reference" compiler. With several version of it for the same test case actually :)

I just fix both errors. 😅 That's why I just assumed this was related to sneaky throws—the reason this came up is that because of FauxPas, I should have no errors (as the Open JDK does), but in Eclipse I get one error. (That's also why it confused me when you were talking about two versus one error.)

Let us be sure we are on the same page: The one error you get with Eclipse is when the commented line

//TODO uncomment this line to see the problem:             document = documentBuilder.parse(inputStream);

is uncommented - right ?

When it stands commented out, the behavior matches javac (no errors) Right ?

If you compile the final form of the snippet I arrived at with javac, you will see one error, which is also complained about by ECJ - I think when you run it under the right frameworks this error goes away due to the sneaky throw trick is my understanding.

@garretwilson
Copy link
Author

Let us be sure we are on the same page: The one error you get with Eclipse is when the commented line //TODO uncomment … is uncommented - right ?

Right!

When it stands commented out, the behavior matches javac (no errors) Right ?

Exactly. Because of Faux Pas, there are no errors. (I can even say throw new IOException(…) and it doesn't complain.) Neither Eclipse nor Open JDK show any errors with the //TODO uncomment line commented. As soon as I uncomment that line, Eclipse shows one error, and Open JDK continues to show no errors when compiling.

It's getting bedtime here, so I'll happily answer any more questions tomorrow. Thanks for taking an interest in this ticket.

@srikanth-sankaran
Copy link
Contributor

Further simplified test case:

import java.io.IOException;
import java.nio.file.Path;
import java.util.function.Consumer;
import java.util.stream.Stream;

public class X11 {

	public X11 parse() throws IOException {
		return null;
	}
	
	Stream<Path> list() throws IOException {
		return null;
	}

	public void foo() throws IOException {

		throwingConsumer((Path barDir) -> {
			try (final Stream<Path> files = list()) {
				throwingConsumer((Path file) -> {
					final X11 document;
					document = parse();
				});
			}
		});
	}

	public static <T, X extends Throwable> ThrowingConsumer<T, X> throwingConsumer(
			final ThrowingConsumer<T, X> consumer) {
		return consumer;
	}
}


interface ThrowingConsumer<T, X extends Throwable> extends Consumer<T> {

	void tryAccept(T t) throws X;

	default void accept(final T t) {
		tryAccept(t);
	}
}

Interestingly, if the innermost

final X11 document;
document = parse();

were declared to be either:

final X11 document = parse();

or

X11 document;
document = parse();

the problem goes away

@srikanth-sankaran srikanth-sankaran self-assigned this Jun 20, 2023
@srikanth-sankaran srikanth-sankaran added this to the 4.29 M1 milestone Jun 20, 2023
@srikanth-sankaran
Copy link
Contributor

Problem originates at this method org.eclipse.jdt.internal.compiler.flow.UnconditionalFlowInfo.fakeInitializedFlowInfo(int, int)
whose stated intent is:

fakeInitializedFlowInfo: For Lambda expressions tentative analysis during overload resolution.
   We presume that any and all outer locals touched by the lambda are definitely assigned and
   effectively final. Whether they are or not is immaterial for overload analysis (errors encountered
   in the body are not supposed to influence the resolution. It is pertinent only for the eventual
   resolution/analysis post overload resolution. For lambda's the problem is that we start the control/data
   flow analysis abruptly at the start of the lambda, so we need to present a cogent world view and hence
   all this charade.

But caller is passing this.scope.outerMostMethodScope().analysisIndex; for localCount parameter which includes the locals of the lambda - not just its outer locals. Bad things happen from there (duplicate initialization error, which gets tagged as ignored mandatory error, precluding the exception analysis of the outer lambda)

@srikanth-sankaran
Copy link
Contributor

It is worrisome that org.eclipse.jdt.internal.compiler.lookup.MethodScope.analysisIndex is monotonously increasing and for resolution of copies the indices are not overlaid. I don't know what problem could arise out of it - but we have had this behavior for nearly a decade now :)

srikanth-sankaran added a commit to srikanth-sankaran/eclipse.jdt.core that referenced this issue Jun 20, 2023
* The method
org.eclipse.jdt.internal.compiler.ast.LambdaExpression.analyzeExceptions()
over-reaches into current lambda locals while wanting to flag all outer
locals touched by the lambda as being definitely assigned and
   effectively final.

* Fixes eclipse-jdt#1162
srikanth-sankaran added a commit to srikanth-sankaran/eclipse.jdt.core that referenced this issue Jun 20, 2023
* The method
org.eclipse.jdt.internal.compiler.ast.LambdaExpression.analyzeExceptions()
over-reaches into current lambda locals while wanting to flag all outer
locals touched by the lambda as being definitely assigned and
   effectively final.

* Fixes eclipse-jdt#1162
@srikanth-sankaran
Copy link
Contributor

The PR #1166 fixes problem in the reduced test case - If you are able to verify the fix against the actual set up and report feedback, that would be great. Not sure if you are set up to do that without the PR being merged in - by applying patches and such. TIA

@garretwilson
Copy link
Author

garretwilson commented Jun 20, 2023

Problem originates at this method …

Woohoo!! 🥳 As soon as I looked at my previous Eclipse compiler tickets and realized you were the one who worked on them before, I knew that with your expertise you were the right person to work on this! Thanks, @srikanth-sankaran .

If you are able to verify the fix against the actual set up and report feedback, that would be great.

I confess that I won't be able to find the time to apply patches and such, but listen, I'll try to install one of the Eclipse 2023-09 milestones when they come out (even though I'm leery of the new bugs they will introduce) and see if these changes do the trick on my actual project.

Thanks for looking into this and addressing it so quickly!

@stephan-herrmann
Copy link
Contributor

It is worrisome that org.eclipse.jdt.internal.compiler.lookup.MethodScope.analysisIndex is monotonously increasing and for resolution of copies the indices are not overlaid. I don't know what problem could arise out of it - but we have had this behavior for nearly a decade now :)

Why do you worry? Monotony ensures uniqueness, so we don't mix the universes of different lambda copies, although the outermost method is not copied. Also this index is only used internally, mainly for LocalVariableBinding.id, which is not the position used during code gen, i.e., byte code is unaffected. The only risk I could think of is overflow if a lambda is copied gazillion times :)

srikanth-sankaran added a commit to srikanth-sankaran/eclipse.jdt.core that referenced this issue Jun 29, 2023
* The method
org.eclipse.jdt.internal.compiler.ast.LambdaExpression.analyzeExceptions()
over-reaches into current lambda locals while wanting to flag all outer
locals touched by the lambda as being definitely assigned and
   effectively final.

* Fixes eclipse-jdt#1162
srikanth-sankaran added a commit to srikanth-sankaran/eclipse.jdt.core that referenced this issue Jun 30, 2023
subyssurendran666 pushed a commit to subyssurendran666/eclipse.jdt.core that referenced this issue Jul 18, 2023
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.

3 participants