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

Improve JavacBindingResolver #909

Merged
merged 1 commit into from
Oct 29, 2024

Conversation

snjeza
Copy link

@snjeza snjeza commented Oct 28, 2024

Related to #755

@snjeza snjeza changed the title Improve JavacBindingResolver [WIP] Improve JavacBindingResolver Oct 28, 2024
@snjeza
Copy link
Author

snjeza commented Oct 28, 2024

de42c03#diff-af030ac91c9fb35cb660aa9076bd04fcbfde2ebc73c583664b39293fc8475e3bR449-R455 doubles the speed of the resolve method.

@datho7561
Copy link

datho7561 commented Oct 29, 2024

There are around 13 new failures, I think these are them:

org.eclipse.jdt.core.tests.dom.ASTConverter18Test.testBug433879c
org.eclipse.jdt.core.tests.dom.ASTConverter18Test.testBug433879d
org.eclipse.jdt.core.tests.dom.ASTConverterAST3Test.test0261
org.eclipse.jdt.core.tests.dom.ASTConverterAST4Test.test0261
org.eclipse.jdt.core.tests.dom.ASTConverterAST8Test.test0261
org.eclipse.jdt.core.tests.dom.ASTConverterTest2.test0438
org.eclipse.jdt.core.tests.dom.ASTConverterTestAST3_2.test0438
org.eclipse.jdt.core.tests.dom.ASTConverterTestAST3_2.test0667_2
org.eclipse.jdt.core.tests.dom.ASTConverterTestAST4_2.test0438
org.eclipse.jdt.core.tests.dom.ASTConverterTestAST4_2.test0667_2
org.eclipse.jdt.core.tests.dom.ASTConverterTestAST8_2.test0438
org.eclipse.jdt.core.tests.dom.ASTConverterTestAST8_2.test0667_2
org.eclipse.jdt.core.tests.dom.ASTConverterTest.test0261

@datho7561
Copy link

These two tests are invalid argument exceptions thrown by javac:

org.eclipse.jdt.core.tests.dom.ASTConverter18Test.testBug433879c
org.eclipse.jdt.core.tests.dom.ASTConverter18Test.testBug433879d

I think they are worth investigating further, because perhaps we are passing in bad values to javac.

The other failures appear to be changes in the number of errors reported by javac. I am less worried about those ones.

Copy link

@datho7561 datho7561 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems good to me. Thanks, Snjezana!

@snjeza snjeza changed the title [WIP] Improve JavacBindingResolver Improve JavacBindingResolver Oct 29, 2024
@datho7561 datho7561 merged commit 81d96cc into eclipse-jdtls:dom-with-javac Oct 29, 2024
4 of 5 checks passed
@mickaelistria
Copy link

The other failures appear to be changes in the number of errors reported by javac. I am less worried about those ones.

I was worried but debugging shows that now with this change, the UnusedTreeScanner is run although there are errors in enter instead of being interrupted earlier. I think it's a good thing. If it becomes bogus, we'll have to add a check before running .analyze(tree), but so far so good. Thanks!

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.

3 participants