Skip to content

Don't report AbortCompilation exceptions from parser#4808

Merged
iloveeclipse merged 1 commit intoeclipse-jdt:masterfrom
iloveeclipse:issue_2769_alternative
Jan 30, 2026
Merged

Don't report AbortCompilation exceptions from parser#4808
iloveeclipse merged 1 commit intoeclipse-jdt:masterfrom
iloveeclipse:issue_2769_alternative

Conversation

@iloveeclipse
Copy link
Member

Regression from
#4451 & #4302.

If Java Editor interrupts hover computation, nothing should be reported, as it is expected. After changes above two errors are reported into the log (with same stack).

Fixes eclipse-jdt/eclipse.jdt.ui#2769

Regression from
eclipse-jdt#4451 &
eclipse-jdt#4302.

If Java Editor interrupts hover computation, nothing should be reported,
as it is expected. After changes above two errors are reported into the
log (with same stack).

Fixes eclipse-jdt/eclipse.jdt.ui#2769
@stephan-herrmann
Copy link
Contributor

This looks well aligned with existing stuff!

By debugging tests like org.eclipse.jdt.core.tests.dom.ASTConverterTestAST3_2.test0521(), I could observe that the very location of your fix may already throw OperationCanceledException, which is wrapped in AbortCompilation in this call stack:

AbortCompilation.<init>(boolean, RuntimeException) line: 57	
CancelableNameEnvironment.checkCanceled() line: 40	
CancelableNameEnvironment.findType(char[], char[][]) line: 52	
LookupEnvironment.askForType(PackageBinding, char[], ModuleBinding) line: 386	
PlainPackageBinding(PackageBinding).getTypeOrPackage(char[], ModuleBinding, boolean) line: 277	
CompilationUnitScope.getDefaultImports() line: 761	
CompilationUnitScope.checkAndSetImports() line: 234	
LookupEnvironment$CompleteTypeBindingsSteps.perform(CompilationUnitScope) line: 220	
LookupEnvironment.completeTypeBindings() line: 612	
CompilationUnitResolver(Compiler).internalBeginToCompile(ICompilationUnit[], int) line: 787	
CompilationUnitResolver(Compiler).beginToCompile(ICompilationUnit[]) line: 277	
CompilationUnitResolver.resolve(CompilationUnitDeclaration, ICompilationUnit, NodeSearcher, boolean, boolean, boolean) line: 1240	
CompilationUnitResolver.resolve(ICompilationUnit, IJavaProject, List, NodeSearcher, Map, WorkingCopyOwner, int, IProgressMonitor) line: 830	
CompilationUnitResolver.toCompilationUnit(ICompilationUnit, boolean, IJavaProject, List<Classpath>, NodeSearcher, int, Map<String,String>, WorkingCopyOwner, WorkingCopyOwner, int, IProgressMonitor) line: 1405	
CompilationUnitResolver$ECJCompilationUnitResolver.toCompilationUnit(ICompilationUnit, boolean, IJavaProject, List<Classpath>, int, int, Map<String,String>, WorkingCopyOwner, WorkingCopyOwner, int, IProgressMonitor) line: 109	
ASTParser.internalCreateASTCached(IProgressMonitor) line: 1410	
ASTParser.lambda$1(IProgressMonitor) line: 1289	
Lambda.call() line: not available	
JavaModelManager.cacheZipFiles(JavaCallable<T,E>) line: 5709	
ASTParser.internalCreateAST(IProgressMonitor) line: 1289	
ASTParser.createAST(IProgressMonitor) line: 933	
ASTConverterTestAST3_2.test0521() line: 3491	

So adding another scenario to the same policy looks safe.

And in the scenario from eclipse-jdt/eclipse.jdt.ui#2769 this OperationCanceledException will be explicitly caught from the try-catch surrounding:

at org.eclipse.jdt.core.manipulation.CoreASTProvider$1.run(CoreASTProvider.java:294)

While looking left and right at other silent AbortCompilation, I wondered if this guy should be improved, too: org.eclipse.jdt.internal.compiler.ProcessTaskManager.removeNextUnits() throws AbortCompilation like this:

			} catch (InterruptedException interrupt) {
				throw new AbortCompilation(true/* silent */, new RuntimeException(interrupt));
			}

This looks similar to the Parser use case, doesn't it? Could we think of a situation where this new RuntimeException(interrupt) could be useful? Or should we change this to null like in the Parser case? I see you already had a discussion concerning this very line in #2948 (comment) 😄

I'll leave it to you, if you want to add a change for ProcessTaskManager. Otherwise the proposed change already looks good to me.

@iloveeclipse
Copy link
Member Author

While looking left and right at other silent AbortCompilation, I wondered if this guy should be improved, too: org.eclipse.jdt.internal.compiler.ProcessTaskManager.removeNextUnits() throws AbortCompilation like this:

			} catch (InterruptedException interrupt) {
				throw new AbortCompilation(true/* silent */, new RuntimeException(interrupt));
			}

This looks similar to the Parser use case, doesn't it? Could we think of a situation where this new RuntimeException(interrupt) could be useful? Or should we change this to null like in the Parser case? I see you already had a discussion concerning this very line in #2948 (comment) 😄

This opens another can of worms.
If I see it right, the AbortCompilation from that code is re-thrown here:

try {
units = processingTask.removeNextUnits();
} catch (Error | RuntimeException e) {
unit = processingTask.getUnitWithError();
throw e;

immediately handled here in same method:

} catch (AbortCompilation e) {
this.handleInternalException(e, unit);
} catch (Error | RuntimeException e) {

and the wrapped RuntimeException is then re-thrown here:

protected void handleInternalException(
AbortCompilation abortException,
CompilationUnitDeclaration unit) {
/* special treatment for SilentAbort: silently cancelling the compilation process */
if (abortException.isSilent) {
if (abortException.silentException == null) {
return;
}
throw abortException.silentException;

... and then it looks like no one handles this exception properly, we have lot of callers here:

image

@iloveeclipse iloveeclipse merged commit 857bcd5 into eclipse-jdt:master Jan 30, 2026
13 checks passed
@iloveeclipse iloveeclipse deleted the issue_2769_alternative branch January 30, 2026 10:23
@iloveeclipse
Copy link
Member Author

Follow up: #4809

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 this pull request may close these issues.

CoreASTProvider.createAST reports errors on interruption

2 participants

Comments