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

fix: Fix compliance level support for old code #4068

Merged
merged 3 commits into from
Aug 9, 2021

Conversation

andre15silva
Copy link
Contributor

@andre15silva andre15silva commented Jul 27, 2021

This PR fixes #4067

Added a test case similar to the problem encountered with the example from astor.

Copy link
Collaborator

@SirYwell SirYwell left a comment

Choose a reason for hiding this comment

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

In regards of #4072, I've run the CtGenerationTest with your changes too, and it takes around 8-10s more time than with the master branch (on my machine).

It would be probably better to keep the static keyword set but remove the version specific ones and add checks for them in the isKeyword method directly.

@andre15silva
Copy link
Contributor Author

In regards of #4072, I've run the CtGenerationTest with your changes too, and it takes around 8-10s more time than with the master branch (on my machine).

It would be probably better to keep the static keyword set but remove the version specific ones and add checks for them in the isKeyword method directly.

Hi @SirYwell

You're absolutely right. Thank you and sorry for the regression.

I've updated the PR, it should be fixed now.

Signed-off-by: André Silva <[email protected]>
@monperrus
Copy link
Collaborator

Nice fix, very helpful to analyze old Java code, which is definitely in our scope.

Will merge once CI is green.

Thanks @andre15silva

@andre15silva
Copy link
Contributor Author

Green @monperrus

@monperrus monperrus changed the title Fix keyword list not accounting for compliance level fix: Fix compliance level support for old code Aug 9, 2021
@monperrus monperrus merged commit b80856b into INRIA:master Aug 9, 2021
@monperrus monperrus mentioned this pull request Aug 19, 2021
woutersmeenk pushed a commit to woutersmeenk/spoon that referenced this pull request Aug 29, 2021
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.

Compliance level is not used in keyword constraint
3 participants