JDK-8290901: Reduce use of -source in langtools tests #9622

jddarcy · 2022-07-23T02:29:56Z

To a first approximation, this change replaces use of "-source", where possible, with the preferable "--release" javac option. (There are cases where this swap is not desirable, such as when newer library features are being used in the test.)

A few unneeded uses of -source were removed entirely.

I'll update copyrights before any push.

Progress

Change must be properly reviewed (1 review required, with at least 1 Reviewer)
Change must not contain extraneous whitespace
Commit message must refer to an issue

Issue

JDK-8290901: Reduce use of -source in langtools tests

Reviewers

Jonathan Gibbons (@jonathan-gibbons - Reviewer)

Reviewing

Using git

Checkout this PR locally:
$ git fetch https://git.openjdk.org/jdk pull/9622/head:pull/9622
$ git checkout pull/9622

Update a local copy of the PR:
$ git checkout pull/9622
$ git pull https://git.openjdk.org/jdk pull/9622/head

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 9622

View PR using the GUI difftool:
$ git pr show -t 9622

Using diff file

Download this PR as a diff file:
https://git.openjdk.org/jdk/pull/9622.diff

bridgekeeper · 2022-07-23T02:30:40Z

👋 Welcome back darcy! A progress list of the required criteria for merging this PR into master will be added to the body of your pull request. There are additional pull request commands available for use with this pull request.

openjdk · 2022-07-23T02:32:59Z

@jddarcy The following label will be automatically applied to this pull request:

compiler

When this pull request is ready to be reviewed, an "RFR" email will be sent to the corresponding mailing list. If you would like to change these labels, use the /label pull request command.

mlbridge · 2022-07-23T02:35:48Z

Webrevs

jonathan-gibbons

I think you can probably simply and sometimes delete the -Xlint option ... specifically, to remove the -options sub-option.

jonathan-gibbons · 2022-07-28T22:11:28Z

test/langtools/tools/javac/generics/7015430/T7015430.java

@@ -4,8 +4,8 @@
 *
 * @summary  Incorrect thrown type determined for unchecked invocations
 * @author Daniel Smith
- * @compile/fail/ref=T7015430_1.out -source 7 -Xlint:-options,unchecked -XDrawDiagnostics T7015430.java
- * @compile/fail/ref=T7015430_2.out -Xlint:unchecked -XDrawDiagnostics T7015430.java
+ * @compile/fail/ref=T7015430_1.out --release 7 -Xlint:-options,unchecked -XDrawDiagnostics T7015430.java


You may be able to remove -options from -Xlint -- the value is probably there to suppress the warnings that you have deleted elsewhere

There are warnings for using "--release 7" because of the age of 7, but otherwise I've updated the PR to remove unneeded uses of -Xlint:-options.

jonathan-gibbons · 2022-07-28T22:13:18Z

test/langtools/tools/javac/generics/diamond/neg/Neg09a.java

@@ -4,8 +4,7 @@
 *
 * @summary  Check that diamond is not allowed with anonymous inner class expressions at source < 9
 * @author Maurizio Cimadamore
- * @compile/fail/ref=Neg09a.out Neg09a.java -source 8 -XDrawDiagnostics -Xlint:-options
- *
+ * @compile/fail/ref=Neg09a.out Neg09a.java --release 8 -XDrawDiagnostics -Xlint:-options


another -options

test/langtools/tools/javac/generics/diamond/neg/Neg09b.java

test/langtools/tools/javac/generics/diamond/neg/Neg09c.java

jonathan-gibbons · 2022-07-28T22:16:14Z

test/langtools/tools/javac/generics/diamond/neg/Neg09d.java

@@ -4,8 +4,7 @@
 *
 * @summary  Check that diamond is not allowed with anonymous inner class expressions at source < 9
 * @author Maurizio Cimadamore
- * @compile/fail/ref=Neg09d.out Neg09d.java -source 8 -XDrawDiagnostics -Xlint:-options
- *
+ * @compile/fail/ref=Neg09d.out Neg09d.java --release 8 -XDrawDiagnostics -Xlint:-options


test/langtools/tools/javac/generics/diamond/neg/Neg10.java

test/langtools/tools/javac/generics/inference/7154127/T7154127.java

test/langtools/tools/javac/generics/inference/7177306/T7177306e.java

test/langtools/tools/javac/generics/inference/8015505/T8015505.java

jonathan-gibbons

generally, a nice cleanup, but check out NestedCapture.java

jonathan-gibbons · 2022-07-29T16:15:01Z

test/langtools/tools/javac/generics/inference/NestedCapture.java

 * @compile NestedCapture.java
- * @compile -Xlint:-options -source 7 NestedCapture.java
+ * @compile NestedCapture.java


Did too much get removed here? Should there be a --release 7?

Good catch; fixed.

jonathan-gibbons

You probably don't need the -Xlint:-options in this case, because you are not checking the output with a reference output file, so the warning is mostly harmless.

I'll approve the review and leave it to you to decide whether to suppress the -Xlint:-options

openjdk · 2022-07-29T17:30:16Z

@jddarcy This change now passes all automated pre-integration checks.

ℹ️ This project also has non-automated pre-integration requirements. Please see the file CONTRIBUTING.md for details.

After integration, the commit message for the final commit will be:

8290901: Reduce use of -source in langtools tests

Reviewed-by: jjg

You can use pull request commands such as /summary, /contributor and /issue to adjust it as needed.

At the time when this comment was updated there had been 7 new commits pushed to the master branch:

64a1a08: 8289647: AssertionError during annotation processing of record related tests
95fc16b: 8290525: Move HeapRegion::_compaction_top to G1FullCollector
f58e08e: 8290715: Fix incorrect uses of G1CollectedHeap::heap_region_containing()
18cd16d: 8291003: ARM32: constant_table.size assertion
cfe9026: 8289511: Improve test coverage for XPath Axes: child
eeac3da: 8289755: Remove --enable-reproducible-build from jib profile
791fc57: 6383195: javax.crypto.spec.PBEKeySpec is not thread safe

Please see this link for an up-to-date comparison between the source branch of this pull request and the master branch.
As there are no conflicts, your changes will automatically be rebased on top of these commits when integrating. If you prefer to avoid this automatic rebasing, please check the documentation for the /integrate command for further details.

➡️ To integrate this PR with the above commit message to the master branch, type /integrate in a new comment.

jddarcy · 2022-07-29T17:32:38Z

You probably don't need the -Xlint:-options in this case, because you are not checking the output with a reference output file, so the warning is mostly harmless.

I'll approve the review and leave it to you to decide whether to suppress the -Xlint:-options

Yes; the test passes with or without -Xlint:-options. I left -options in to avoid spurious messages in the test output and for consistency with the other uses of --release 7, many of which have -options.

jddarcy · 2022-07-29T17:33:43Z

/integrate

openjdk · 2022-07-29T17:35:30Z

Going to push as commit cc2861a.
Since your change was applied there have been 7 commits pushed to the master branch:

64a1a08: 8289647: AssertionError during annotation processing of record related tests
95fc16b: 8290525: Move HeapRegion::_compaction_top to G1FullCollector
f58e08e: 8290715: Fix incorrect uses of G1CollectedHeap::heap_region_containing()
18cd16d: 8291003: ARM32: constant_table.size assertion
cfe9026: 8289511: Improve test coverage for XPath Axes: child
eeac3da: 8289755: Remove --enable-reproducible-build from jib profile
791fc57: 6383195: javax.crypto.spec.PBEKeySpec is not thread safe

Your commit was automatically rebased without conflicts.

openjdk · 2022-07-29T17:35:42Z

@jddarcy Pushed as commit cc2861a.

💡 You may see a message that your pull request was closed with unmerged commits. This can be safely ignored.

JDK-8290901: Reduce use of -source in langtools tests

63e95cf

openjdk bot added the rfr Pull request is ready for review label Jul 23, 2022

openjdk bot added the compiler compiler-dev@openjdk.org label Jul 23, 2022

jddarcy added 2 commits July 25, 2022 10:28

Update copyrights.

a375ff8

Merge branch 'master' into JDK-8290901

a743833

jonathan-gibbons reviewed Jul 28, 2022

View reviewed changes

jddarcy added 2 commits July 28, 2022 17:14

Remove unneeded -Xlint:-options flag

3a6cb66

Remove one more unneeded -Xlint:-options usage.

f24c3f6

jonathan-gibbons reviewed Jul 29, 2022

View reviewed changes

Fix bug found in code review.

5455476

jonathan-gibbons reviewed Jul 29, 2022

View reviewed changes

jonathan-gibbons approved these changes Jul 29, 2022

View reviewed changes

openjdk bot added the ready Pull request is ready to be integrated label Jul 29, 2022

openjdk bot added the integrated Pull request has been integrated label Jul 29, 2022

openjdk bot closed this Jul 29, 2022

openjdk bot removed ready Pull request is ready to be integrated rfr Pull request is ready for review labels Jul 29, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

JDK-8290901: Reduce use of -source in langtools tests #9622

JDK-8290901: Reduce use of -source in langtools tests #9622

jddarcy commented Jul 23, 2022 •

edited by openjdk bot

Loading

bridgekeeper bot commented Jul 23, 2022

openjdk bot commented Jul 23, 2022

mlbridge bot commented Jul 23, 2022 •

edited

Loading

jonathan-gibbons left a comment

jonathan-gibbons Jul 28, 2022

jddarcy Jul 29, 2022

jonathan-gibbons Jul 28, 2022

jonathan-gibbons Jul 28, 2022

jonathan-gibbons left a comment

jonathan-gibbons Jul 29, 2022

jddarcy Jul 29, 2022

jonathan-gibbons left a comment

openjdk bot commented Jul 29, 2022

jddarcy commented Jul 29, 2022

jddarcy commented Jul 29, 2022

openjdk bot commented Jul 29, 2022

openjdk bot commented Jul 29, 2022

JDK-8290901: Reduce use of -source in langtools tests #9622

JDK-8290901: Reduce use of -source in langtools tests #9622

Conversation

jddarcy commented Jul 23, 2022 • edited by openjdk bot Loading

Progress

Issue

Reviewers

Reviewing

bridgekeeper bot commented Jul 23, 2022

openjdk bot commented Jul 23, 2022

mlbridge bot commented Jul 23, 2022 • edited Loading

Webrevs

jonathan-gibbons left a comment

Choose a reason for hiding this comment

jonathan-gibbons Jul 28, 2022

Choose a reason for hiding this comment

jddarcy Jul 29, 2022

Choose a reason for hiding this comment

jonathan-gibbons Jul 28, 2022

Choose a reason for hiding this comment

jonathan-gibbons Jul 28, 2022

Choose a reason for hiding this comment

jonathan-gibbons left a comment

Choose a reason for hiding this comment

jonathan-gibbons Jul 29, 2022

Choose a reason for hiding this comment

jddarcy Jul 29, 2022

Choose a reason for hiding this comment

jonathan-gibbons left a comment

Choose a reason for hiding this comment

openjdk bot commented Jul 29, 2022

jddarcy commented Jul 29, 2022

jddarcy commented Jul 29, 2022

openjdk bot commented Jul 29, 2022

openjdk bot commented Jul 29, 2022

jddarcy commented Jul 23, 2022 •

edited by openjdk bot

Loading

mlbridge bot commented Jul 23, 2022 •

edited

Loading