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

ASTConverterSuperAfterStatements.test004 fails on master #2187

Closed
srikanth-sankaran opened this issue Mar 20, 2024 · 13 comments · Fixed by #2225
Closed

ASTConverterSuperAfterStatements.test004 fails on master #2187

srikanth-sankaran opened this issue Mar 20, 2024 · 13 comments · Fixed by #2225
Assignees

Comments

@srikanth-sankaran
Copy link
Contributor

Seems to trigger the new warning:

1. WARNING in /Converter_22/src/X.java (at line 2)\n
 X() {\n
 ^^^\n
Internal inconsistency: Inappropriate operand stack size encountered during translation\n
2. WARNING in /Converter_22/src/X.java (at line 2)\n
 X() {\n
 ^^^\n
Internal inconsistency: Inappropriate operand stack size encountered during translation\n
@srikanth-sankaran srikanth-sankaran self-assigned this Mar 20, 2024
@srikanth-sankaran
Copy link
Contributor Author

@jarthana, @mpalat - I am following up.

@srikanth-sankaran
Copy link
Contributor Author

srikanth-sankaran commented Mar 21, 2024

Code generated for the constructor is

      stack=2, locals=2, args_size=1
         0: new           #8                  // class X$1
         3: dup
         4: invokespecial #10                 // Method X$1."<init>":()V
         7: astore_1
         8: aload_0
         9: invokespecial #12                 // Method java/lang/Object."<init>":()V
        12: return

In the call stack below, we compute the receiverAndArgsSize to be 2 - that looks suspect to me compared to the signature of the <init> method invokespecialed which is **()V**

TypeAnnotationCodeStream(CodeStream).invoke(byte, MethodBinding, TypeBinding, TypeReference[]) line: 4785	
TypeAnnotationCodeStream.invoke(byte, MethodBinding, TypeBinding, TypeReference[]) line: 132	
QualifiedAllocationExpression.generateCode(BlockScope, CodeStream, boolean) line: 220	
LocalDeclaration.generateCode(BlockScope, CodeStream) line: 174	
ConstructorDeclaration.internalGenerateCode(ClassScope, ClassFile) line: 459	
ConstructorDeclaration.generateCode(ClassScope, ClassFile) line: 310	
TypeDeclaration.generateCode(ClassFile) line: 756	
TypeDeclaration.generateCode(CompilationUnitScope) line: 826	
CompilationUnitDeclaration.generateCode() line: 412	
CompilationUnitProblemFinder(Compiler).resolve(CompilationUnitDeclaration, ICompilationUnit, boolean, boolean, boolean) line: 1074	
CompilationUnitProblemFinder.process(CompilationUnit, SourceElementParser, WorkingCopyOwner, Map<String,CategorizedProblem[]>, boolean, int, IProgressMonitor) line: 272	
CompilationUnit.buildStructure(OpenableElementInfo, IProgressMonitor, Map<IJavaElement,IElementInfo>, IResource) line: 186	
CompilationUnit(Openable).generateInfos(IElementInfo, Map<IJavaElement,IElementInfo>, IProgressMonitor) line: 245	
CompilationUnit(JavaElement).openWhenClosed(IElementInfo, boolean, IProgressMonitor) line: 585	
CompilationUnit.makeConsistent(int, boolean, int, Map<String,CategorizedProblem[]>, IProgressMonitor) line: 1137	
ReconcileWorkingCopyOperation.makeConsistent(CompilationUnit) line: 166	
ReconcileWorkingCopyOperation.executeOperation() line: 92	
ReconcileWorkingCopyOperation(JavaModelOperation).run(IProgressMonitor) line: 740	
ReconcileWorkingCopyOperation(JavaModelOperation).runOperation(IProgressMonitor) line: 805	
CompilationUnit.reconcile(int, int, WorkingCopyOwner, IProgressMonitor) line: 1311	
ASTConverterSuperAfterStatements(AbstractASTTests).buildASTs(int, String, ICompilationUnit, boolean, boolean, boolean) line: 464	
ASTConverterSuperAfterStatements(AbstractASTTests).buildASTs(String, ICompilationUnit, boolean, boolean, boolean) line: 530	
ASTConverterSuperAfterStatements(AbstractASTTests).buildAST(String, ICompilationUnit, boolean, boolean, boolean) line: 378	
ASTConverterSuperAfterStatements(AbstractASTTests).buildAST(String, ICompilationUnit, boolean) line: 369	
ASTConverterSuperAfterStatements(AbstractASTTests).buildAST(String, ICompilationUnit) line: 356	
ASTConverterSuperAfterStatements.test004() line: 224	

In short I think the warning is good and is pointing to a problem in the product.

@srikanth-sankaran
Copy link
Contributor Author

In this call stack below codeStream.stackDepth becomes -1 which is illegal.

TypeAnnotationCodeStream(CodeStream).astore_1() line: 552	
TypeAnnotationCodeStream(CodeStream).store(LocalVariableBinding, boolean) line: 7528	
LocalDeclaration.generateCode(BlockScope, CodeStream) line: 182	
ConstructorDeclaration.internalGenerateCode(ClassScope, ClassFile) line: 459	
ConstructorDeclaration.generateCode(ClassScope, ClassFile) line: 310	
TypeDeclaration.generateCode(ClassFile) line: 756	
TypeDeclaration.generateCode(CompilationUnitScope) line: 826	
CompilationUnitDeclaration.generateCode() line: 412	

@srikanth-sankaran
Copy link
Contributor Author

Manoj, please follow up. Stack traces above point to the problem spots.

@mpalat
Copy link
Contributor

mpalat commented Mar 21, 2024

Manoj, please follow up. Stack traces above point to the problem spots.

Sure Srikanth.. let me take a look

@srikanth-sankaran
Copy link
Contributor Author

I suggest you disable the test by prefixing with _ and merge a commit to master - otherwise we are going to see a failing tests on all PRs until this is resolved. Thanks. Not just jdt/core others will see this failure too and may be puzzled: see #2194

@stephan-herrmann
Copy link
Contributor

Found you! To check why my PR build failed I incrementally merged commits from master into BETA_JAVA22. It was when I merged #2171 that this test started to fail. Looks like this is old news for you guys :)

On the way I was puzzled about the relation between BETA_JAVA22 and #2181: Neither EGit nor github seems to show any relation although reportedly the branch was merged, Egit shows it as if the entire branch BETA_JAVA22 was squashed into a single commit??
Anybody else seeing this, too?
Right when I was about to complain, I discovered that blame still works across this missing merge-link. How is that possible?

@srikanth-sankaran
Copy link
Contributor Author

@jarthana could you answer this question please ? Thanks.

@stephan-herrmann
Copy link
Contributor

On the way I was puzzled about the relation between BETA_JAVA22 and #2181: Neither EGit nor github seems to show any relation although reportedly the branch was merged, Egit shows it as if the entire branch BETA_JAVA22 was squashed into a single commit??

Never mind, my EGit history view had "Show first parent only" enabled (don't know why). Disabling that option revealed the missing link:
image

Sorry for the noise.

@stephan-herrmann
Copy link
Contributor

I suggest you disable the test by prefixing with _ and merge a commit to master - otherwise we are going to see a failing tests on all PRs until this is resolved.

Yes please. All PR builds are failing due to this for more than 4 days now.

@srikanth-sankaran
Copy link
Contributor Author

Hello!

After this fix, I get an AssertionError here:

import java.util.List;
import java.util.ArrayList;

public class X {
	public static void main(String[] args) {
		new X() {
			
		};
	}
	
	void foo() {
		new X() {
			
		};
	}
}

@srikanth-sankaran
Copy link
Contributor Author

Hello!

After this fix, I get an AssertionError here:

import java.util.List;
import java.util.ArrayList;

public class X {
	public static void main(String[] args) {
		new X() {
			
		};
	}
	
	void foo() {
		new X() {
			
		};
	}
}

Please ignore. Dirty workspace ... Sorry about the noise.

@mpalat
Copy link
Contributor

mpalat commented Mar 28, 2024

@srikanth-sankaran
for our debugging on stack errors, we can enable
// debugStackDepth(this.stackDepth);
in CodeStream.popTypeBinding()
piggybacked alongwith this pr. just fyi

robstryker pushed a commit to robstryker/eclipse.jdt.core that referenced this issue Jul 18, 2024
gayanper pushed a commit to gayanper/eclipse.jdt.core that referenced this issue Sep 7, 2024
gayanper pushed a commit to gayanper/eclipse.jdt.core that referenced this issue Sep 7, 2024
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