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

Update TryWithResourcesJavacFilter for javac 11 #669

Merged
merged 1 commit into from
Aug 1, 2018
Merged

Conversation

Godin
Copy link
Member

@Godin Godin commented Apr 3, 2018

javac 11 EA starting from b7 and up to latest (as of today) b24 contains http://hg.openjdk.java.net/jdk/jdk/rev/c2a3a2aa2475 ( JDK-8194978 ), which changes bytecode generation for try-with-resource. Our TryWithResourcesJavacFilter does not detect this new bytecode.

Based on analysis of change, description of desugaring in javadoc of method makeTwrTy in class com/sun/tools/javac/comp/Lower from b24, and practical observations:

  • there is no null check of primaryExc anymore
  • as with javac 9 when resource is initialized with new, then null check of resource is omitted
  • synthetic method $closeResource (that was introduced in javac 9) not generated anymore

To execute our validation tests with javac 11 EA in absence of full support of JDK 11:

$ javac -version
javac 11-ea

$ mvn clean verify \
    -pl org.jacoco.core,org.jacoco.core.test.validation.java7 \
    -Djacoco.skip -Dbytecode.version=10

@Godin Godin self-assigned this Apr 3, 2018
@Godin Godin force-pushed the issue-669 branch 5 times, most recently from 8ebdb99 to 57a2714 Compare April 4, 2018 09:33
@Godin Godin force-pushed the issue-669 branch 2 times, most recently from ec67afd to 9eab568 Compare June 7, 2018 20:43
@Godin Godin added this to TODO in Filtering Jun 7, 2018
@Godin Godin mentioned this pull request Jul 3, 2018
@Godin Godin added this to the 0.8.2 milestone Jul 14, 2018
@Godin Godin force-pushed the issue-669 branch 4 times, most recently from cc2217e to 8c3e0dd Compare July 30, 2018 22:30
@Godin
Copy link
Member Author

Godin commented Jul 30, 2018

@marchof We need to continue work towards JDK 11 support - AFAIK initial release candidate is planned on 2018/08/16. So could you please have a look on this new filter? While it is similar to existing, I think that better to have them separate - existing one is already quite complex.

@Godin Godin requested a review from marchof July 30, 2018 22:31
Copy link
Member

@marchof marchof left a comment

Choose a reason for hiding this comment

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

Looks good to me. Especially as it is properly backed by tests. For final merge the change log should be updated.

I would love to have a more fluent matcher API. Maybe some more isXXX would help to make it more readable. But this is a different story.

@marchof
Copy link
Member

marchof commented Jul 31, 2018

@Godin Would it be already possible to add an JDK 11 build to Travis?

@Godin
Copy link
Member Author

Godin commented Jul 31, 2018

@marchof

change log should be updated

Done, please check.

I would love to have a more fluent matcher API.

Same from my side, but as usual - baby steps, baby steps 😉

Would it be already possible to add an JDK 11 build to Travis?

Definitely! Was planning and will do so right after merge of this PR, so that javac 11 passes our tests 😆 and doesn't lead to build failure.

@marchof marchof merged commit 65d1aea into master Aug 1, 2018
@marchof marchof deleted the issue-669 branch August 1, 2018 15:02
@Godin Godin mentioned this pull request Aug 8, 2018
@Godin Godin moved this from To Do to Done in Filtering Aug 15, 2018
@jacoco jacoco locked as resolved and limited conversation to collaborators May 8, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
Filtering
  
Done
Development

Successfully merging this pull request may close these issues.

None yet

2 participants