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

8244146: javac changes for JEP 306 #3831

Closed
wants to merge 22 commits into from
Closed
Show file tree
Hide file tree
Changes from 11 commits
Commits
Show all changes
22 commits
Select commit Hold shift + click to select a range
1d9f854
8244146: javac changes for JEP 306
jddarcy Apr 21, 2021
15ba103
Merge branch 'master' into 8244146
jddarcy Apr 29, 2021
258241e
8244146: javac changes for JEP 306
jddarcy Apr 30, 2021
1a6a44c
All langtools regression tests pass; switch to filtering out strictfp…
jddarcy May 1, 2021
60084a6
Add explicit test for strictfp-ness checking for methods and construc…
jddarcy May 2, 2021
39395b9
Appease jcheck.
jddarcy May 2, 2021
556ff8d
Checkpoint with initial lint implementation and passing langtools reg…
jddarcy May 4, 2021
7187861
Alternate attempt to get warning suppression working; direct annotati…
jddarcy May 4, 2021
0693cff
Follow Jan's suggestion for diagnostics timing.
jddarcy May 4, 2021
75732d3
Appease jcheck and enabled strictfp in ConstFold.
jddarcy May 4, 2021
a0a68ba
Add explicit test for warning strictfp suppression.
jddarcy May 4, 2021
ec4d163
Respond to review feedback.
jddarcy May 5, 2021
9773a8a
Respond to review feedback.
jddarcy May 5, 2021
008a69e
Respond to review feedback.
jddarcy May 5, 2021
94fe135
Merge branch 'master' into 8244146
jddarcy May 9, 2021
5ba408a
Merge branch 'master' into 8244146
jddarcy May 27, 2021
81ae526
Merge branch 'master' into 8244146
jddarcy May 29, 2021
7fe6d5a
Update GenericStringTest
jddarcy May 29, 2021
0c5417b
Merge branch 'master' into 8244146
jddarcy May 31, 2021
3f1aeec
Merge branch 'master' into 8244146
jddarcy Jun 1, 2021
280c916
Update copyrights.
jddarcy Jun 1, 2021
1d68a28
Fix copyright typo.
jddarcy Jun 1, 2021
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
9 changes: 4 additions & 5 deletions src/java.base/share/classes/java/lang/FdLibm.java
Expand Up @@ -120,7 +120,7 @@ private Cbrt() {
throw new UnsupportedOperationException();
}

public static strictfp double compute(double x) {
public static double compute(double x) {
double t = 0.0;
double sign;

Expand Down Expand Up @@ -203,7 +203,7 @@ private Hypot() {
throw new UnsupportedOperationException();
}

public static strictfp double compute(double x, double y) {
public static double compute(double x, double y) {
double a = Math.abs(x);
double b = Math.abs(y);

Expand Down Expand Up @@ -343,7 +343,7 @@ private Pow() {
throw new UnsupportedOperationException();
}

public static strictfp double compute(final double x, final double y) {
public static double compute(final double x, final double y) {
double z;
double r, s, t, u, v, w;
int i, j, k, n;
Expand Down Expand Up @@ -680,8 +680,7 @@ private Exp() {
throw new UnsupportedOperationException();
}

// should be able to forgo strictfp due to controlled over/underflow
public static strictfp double compute(double x) {
public static double compute(double x) {
double y;
double hi = 0.0;
double lo = 0.0;
Expand Down
13 changes: 4 additions & 9 deletions src/java.base/share/classes/java/lang/StrictMath.java
Expand Up @@ -86,7 +86,6 @@
* @author Joseph D. Darcy
* @since 1.3
*/

public final class StrictMath {

/**
Expand Down Expand Up @@ -207,10 +206,8 @@ private StrictMath() {}
* @return the measurement of the angle {@code angdeg}
* in radians.
*/
public static strictfp double toRadians(double angdeg) {
// Do not delegate to Math.toRadians(angdeg) because
// this method has the strictfp modifier.
return angdeg * DEGREES_TO_RADIANS;
public static double toRadians(double angdeg) {
return Math.toRadians(angdeg);
}

/**
Expand All @@ -224,10 +221,8 @@ public static strictfp double toRadians(double angdeg) {
* @return the measurement of the angle {@code angrad}
* in degrees.
*/
public static strictfp double toDegrees(double angrad) {
// Do not delegate to Math.toDegrees(angrad) because
// this method has the strictfp modifier.
return angrad * RADIANS_TO_DEGREES;
public static double toDegrees(double angrad) {
return Math.toDegrees(angrad);
}

/**
Expand Down
Expand Up @@ -118,6 +118,9 @@ protected Lint(Context context) {
if (source.compareTo(Source.JDK9) >= 0) {
values.add(LintCategory.DEP_ANN);
}
if (source.compareTo(Source.JDK17) >= 0) {
values.add(LintCategory.STRICTFP);
}
values.add(LintCategory.REQUIRES_TRANSITIVE_AUTOMATIC);
values.add(LintCategory.OPENS);
values.add(LintCategory.MODULE);
Expand Down Expand Up @@ -283,6 +286,11 @@ public enum LintCategory {
*/
STATIC("static"),

/**
* Warning about unnecessary uses of the strictfp modifier
jddarcy marked this conversation as resolved.
Show resolved Hide resolved
*/
STRICTFP("strictfp"),

/**
* Warn about synchronization attempts on instances of @ValueBased classes.
*/
Expand Down
Expand Up @@ -224,6 +224,8 @@ public enum Feature {
REIFIABLE_TYPES_INSTANCEOF(JDK16, Fragments.FeatureReifiableTypesInstanceof, DiagKind.PLURAL),
RECORDS(JDK16, Fragments.FeatureRecords, DiagKind.PLURAL),
SEALED_CLASSES(JDK17, Fragments.FeatureSealedClasses, DiagKind.PLURAL),
// todo: will need to supplement/replace with target feature for writing out classes
REDUNDANT_STRICTFP(JDK17),
jddarcy marked this conversation as resolved.
Show resolved Hide resolved
;

enum DiagKind {
Expand Down
19 changes: 19 additions & 0 deletions src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Check.java
Expand Up @@ -1212,6 +1212,9 @@ else if ((sym.owner.flags_field & INTERFACE) != 0)
} else {
mask = MethodFlags;
}
if ((flags & STRICTFP) != 0) {
warnOnExplicitStrictfp(pos, tree);
}
// Imply STRICTFP if owner has STRICTFP set.
if (((flags|implicit) & Flags.ABSTRACT) == 0 ||
((flags) & Flags.DEFAULT) != 0)
Expand Down Expand Up @@ -1253,6 +1256,9 @@ else if ((sym.owner.flags_field & INTERFACE) != 0)
mask &= ~ABSTRACT;
implicit |= FINAL;
}
if ((flags & STRICTFP) != 0) {
warnOnExplicitStrictfp(pos, tree);
}
// Imply STRICTFP if owner has STRICTFP set.
implicit |= sym.owner.flags_field & STRICTFP;
break;
Expand Down Expand Up @@ -1315,6 +1321,19 @@ && checkDisjoint(pos, flags,
return flags & (mask | ~ExtendedStandardFlags) | implicit;
}

private void warnOnExplicitStrictfp(DiagnosticPosition pos, JCTree tree) {
DiagnosticPosition prevLintPos = deferredLintHandler.setPos(tree.pos());
try {
jddarcy marked this conversation as resolved.
Show resolved Hide resolved
deferredLintHandler.report(() -> {
if (lint.isEnabled(LintCategory.STRICTFP)) {
log.warning(LintCategory.STRICTFP,
pos, Warnings.Strictfp); }
});
} finally {
deferredLintHandler.setPos(prevLintPos);
}
}


/** Determine if this enum should be implicitly final.
*
Expand Down
Expand Up @@ -41,6 +41,7 @@
* This code and its internal interfaces are subject to change or
* deletion without notice.</b>
*/
@SuppressWarnings("strictfp")
strictfp class ConstFold {
protected static final Context.Key<ConstFold> constFoldKey = new Context.Key<>();

Expand Down
Expand Up @@ -40,6 +40,7 @@
import com.sun.tools.javac.code.*;
import com.sun.tools.javac.code.Attribute.RetentionPolicy;
import com.sun.tools.javac.code.Directive.*;
import com.sun.tools.javac.code.Source.Feature;
import com.sun.tools.javac.code.Symbol.*;
import com.sun.tools.javac.code.Type.*;
import com.sun.tools.javac.code.Types.SignatureGenerator.InvalidSignatureException;
Expand Down Expand Up @@ -1697,6 +1698,10 @@ protected int writeExtraAttributes(Symbol sym) {
int adjustFlags(final long flags) {
int result = (int)flags;

// Elide strictfp bit in class files
if (Feature.REDUNDANT_STRICTFP.allowedInSource(source))
result = result & ~STRICTFP;

jddarcy marked this conversation as resolved.
Show resolved Hide resolved
if ((flags & BRIDGE) != 0)
result |= ACC_BRIDGE;
if ((flags & VARARGS) != 0)
Expand Down
Expand Up @@ -1764,6 +1764,9 @@ compiler.warn.dir.path.element.not.directory=\
compiler.warn.missing-explicit-ctor=\
class {0} in exported package {1} declares no explicit constructors, thereby exposing a default constructor to clients of module {2}

compiler.warn.strictfp=\
as of release 17, all floating-point expressions are evaluated strictly and ''strictfp'' is not required

jddarcy marked this conversation as resolved.
Show resolved Hide resolved
compiler.warn.finally.cannot.complete=\
finally clause cannot complete normally

Expand Down
Expand Up @@ -246,6 +246,9 @@ javac.opt.Xlint.desc.serial=\
javac.opt.Xlint.desc.static=\
Warn about accessing a static member using an instance.

javac.opt.Xlint.desc.strictfp=\
Warn about unnecessary use of the strictfp modifier.

javac.opt.Xlint.desc.text-blocks=\
Warn about inconsistent white space characters in text block indentation.

Expand Down
4 changes: 2 additions & 2 deletions src/jdk.jshell/share/classes/jdk/jshell/TaskFactory.java
Expand Up @@ -167,7 +167,7 @@ public <Z> Z analyze(Collection<OuterWrap> wraps,
List<String> allOptions = new ArrayList<>();

allOptions.add("--should-stop=at=FLOW");
allOptions.add("-Xlint:unchecked");
allOptions.add("-Xlint:unchecked,-strictfp");
Copy link
Contributor

Choose a reason for hiding this comment

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

I wonder if JShell should also print the strictfp warnings, so that the users would learn about this change. If needed, it should be possible to disable this, I think, possibly like:

diff --git a/test/langtools/jdk/jshell/ClassesTest.java b/test/langtools/jdk/jshell/ClassesTest.java
index 082d757b02f..b0b8156e359 100644
--- a/test/langtools/jdk/jshell/ClassesTest.java
+++ b/test/langtools/jdk/jshell/ClassesTest.java
@@ -342,4 +342,9 @@ public class ClassesTest extends KullaTesting {
         assertEval("new A()");
     }
 
+    @Override
+    public void setUp() {
+        setUp(bc -> bc.compilerOptions("-Xlint:-strictfp"));
+    }
+
 }

allOptions.add("-proc:none");
allOptions.addAll(extraArgs);

Expand All @@ -184,7 +184,7 @@ public <Z> Z compile(Collection<OuterWrap> wraps,

return runTask(wraps.stream(),
sh,
List.of("-Xlint:unchecked", "-proc:none", "-parameters"),
List.of("-Xlint:unchecked,-strictfp", "-proc:none", "-parameters"),
(jti, diagnostics) -> new CompileTask(sh, jti, diagnostics),
worker);
}
Expand Down
Expand Up @@ -26,6 +26,7 @@
* @bug 7166455
* @summary javac doesn't set ACC_STRICT bit on <clinit> for strictfp class
* @modules jdk.jdeps/com.sun.tools.classfile
* @compile -source 16 CheckACC_STRICTFlagOnclinitTest.java
* @run main CheckACC_STRICTFlagOnclinitTest
*/

Expand Down
Expand Up @@ -81,7 +81,8 @@ private void run(JavaCompiler comp)
}

private void compile(JavaCompiler comp) {
JavacTask ct = (JavacTask)comp.getTask(null, null, null, null, null,
JavacTask ct = (JavacTask)comp.getTask(null, null, null,
List.of("--release", "16"), null,
Arrays.asList(source));
try {
if (!ct.call()) {
Expand Down
@@ -1,10 +1,11 @@
/*
* @test /nodynamiccopyright/
* @bug 4153038 4785453
* @bug 4153038 4785453 8244146
* @summary strictfp may not be used with constructors
* @author David Stoutamire (dps)
*
* @compile/fail/ref=BadConstructorModifiers.out -XDrawDiagnostics BadConstructorModifiers.java
* @compile/fail/ref=BadConstructorModifiers.out -XDrawDiagnostics --release 16 BadConstructorModifiers.java
* @compile/fail/ref=BadConstructorModifiers.out -XDrawDiagnostics -Xlint:-strictfp BadConstructorModifiers.java
*/

public class BadConstructorModifiers {
Expand Down
@@ -1,2 +1,2 @@
BadConstructorModifiers.java:12:14: compiler.err.mod.not.allowed.here: strictfp
BadConstructorModifiers.java:13:14: compiler.err.mod.not.allowed.here: strictfp
1 error
2 changes: 1 addition & 1 deletion test/langtools/tools/javac/T6397044.java
Expand Up @@ -49,7 +49,7 @@ public static void main(String[] args) throws Exception {
try (StandardJavaFileManager fm = tool.getStandardFileManager(null, null, null)) {
Iterable<? extends JavaFileObject> files
= fm.getJavaFileObjectsFromFiles(Arrays.asList(new File(srcDir, self + ".java")));
JavacTask task = tool.getTask(null, fm, null, null, null, files);
JavacTask task = tool.getTask(null, fm, null, List.of("--release", "16"), null, files);
Iterable<? extends CompilationUnitTree> trees = task.parse();
Checker checker = new Checker();
for (CompilationUnitTree tree: trees)
Expand Down
@@ -1,8 +1,9 @@
/*
* @test /nodynamiccopyright/
* @bug 8028428
* @bug 8028428 8244146
* @summary Test that only 'public' and 'abstract' elements compile
* @compile/fail/ref=AnnotationTypeElementModifiers.out -XDrawDiagnostics AnnotationTypeElementModifiers.java
* @compile/fail/ref=AnnotationTypeElementModifiers.out -XDrawDiagnostics --release 16 AnnotationTypeElementModifiers.java
* @compile/fail/ref=AnnotationTypeElementModifiers.out -XDrawDiagnostics -Xlint:-strictfp AnnotationTypeElementModifiers.java
*/

public @interface AnnotationTypeElementModifiers {
Expand Down
@@ -1,21 +1,21 @@
AnnotationTypeElementModifiers.java:17:17: compiler.err.mod.not.allowed.here: private
AnnotationTypeElementModifiers.java:18:17: compiler.err.mod.not.allowed.here: private
AnnotationTypeElementModifiers.java:20:19: compiler.err.mod.not.allowed.here: protected
AnnotationTypeElementModifiers.java:19:17: compiler.err.mod.not.allowed.here: private
AnnotationTypeElementModifiers.java:21:19: compiler.err.mod.not.allowed.here: protected
AnnotationTypeElementModifiers.java:23:16: compiler.err.mod.not.allowed.here: static
AnnotationTypeElementModifiers.java:22:19: compiler.err.mod.not.allowed.here: protected
AnnotationTypeElementModifiers.java:24:16: compiler.err.mod.not.allowed.here: static
AnnotationTypeElementModifiers.java:26:15: compiler.err.mod.not.allowed.here: final
AnnotationTypeElementModifiers.java:25:16: compiler.err.mod.not.allowed.here: static
AnnotationTypeElementModifiers.java:27:15: compiler.err.mod.not.allowed.here: final
AnnotationTypeElementModifiers.java:29:22: compiler.err.mod.not.allowed.here: synchronized
AnnotationTypeElementModifiers.java:28:15: compiler.err.mod.not.allowed.here: final
AnnotationTypeElementModifiers.java:30:22: compiler.err.mod.not.allowed.here: synchronized
AnnotationTypeElementModifiers.java:32:18: compiler.err.mod.not.allowed.here: volatile
AnnotationTypeElementModifiers.java:31:22: compiler.err.mod.not.allowed.here: synchronized
AnnotationTypeElementModifiers.java:33:18: compiler.err.mod.not.allowed.here: volatile
AnnotationTypeElementModifiers.java:35:19: compiler.err.mod.not.allowed.here: transient
AnnotationTypeElementModifiers.java:34:18: compiler.err.mod.not.allowed.here: volatile
AnnotationTypeElementModifiers.java:36:19: compiler.err.mod.not.allowed.here: transient
AnnotationTypeElementModifiers.java:38:16: compiler.err.mod.not.allowed.here: native
AnnotationTypeElementModifiers.java:37:19: compiler.err.mod.not.allowed.here: transient
AnnotationTypeElementModifiers.java:39:16: compiler.err.mod.not.allowed.here: native
AnnotationTypeElementModifiers.java:41:20: compiler.err.mod.not.allowed.here: strictfp
AnnotationTypeElementModifiers.java:40:16: compiler.err.mod.not.allowed.here: native
AnnotationTypeElementModifiers.java:42:20: compiler.err.mod.not.allowed.here: strictfp
AnnotationTypeElementModifiers.java:44:17: compiler.err.mod.not.allowed.here: default
AnnotationTypeElementModifiers.java:43:20: compiler.err.mod.not.allowed.here: strictfp
AnnotationTypeElementModifiers.java:45:17: compiler.err.mod.not.allowed.here: default
AnnotationTypeElementModifiers.java:46:17: compiler.err.mod.not.allowed.here: default
20 errors
Expand Up @@ -30,7 +30,7 @@
* jdk.compiler/com.sun.tools.javac.main
* jdk.jdeps/com.sun.tools.javap
* @build toolbox.ToolBox toolbox.JavapTask
* @run compile -g NestedLambdasCastedTest.java
* @run compile -source 16 -g NestedLambdasCastedTest.java
* @run main NestedLambdasCastedTest

Choose a reason for hiding this comment

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

To massage the existing tests at some places you are passing -source 16 and at others -release 16. Is there some nuance behind it ?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes; --release 16 is generally preferred as a cleaner, more complete way of requesting 16-ness from javac. However, use of features like the @modules tag in jtreg can preclude use from --release because of interactions like:

exporting a package from system module jdk.jdeps is not allowed with --release

*/

Expand Down
Expand Up @@ -26,6 +26,7 @@
* @bug 8012723
* @summary strictfp interface misses strictfp modifer on default method
* @modules jdk.jdeps/com.sun.tools.classfile
* @compile -source 16 CheckACC_STRICTFlagOnDefaultMethodTest.java
* @run main CheckACC_STRICTFlagOnDefaultMethodTest
*/

Expand Down
@@ -0,0 +1,28 @@
/*
* Copyright (c) 2021, Oracle and/or its affiliates. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
* under the terms of the GNU General Public License version 2 only, as
* published by the Free Software Foundation.
*
* This code is distributed in the hope that it will be useful, but WITHOUT
* ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
* FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License
* version 2 for more details (a copy is included in the LICENSE file that
* accompanied this code).
*
* You should have received a copy of the GNU General Public License version
* 2 along with this work; if not, write to the Free Software Foundation,
* Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
*
* Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
* or visit www.oracle.com if you need additional information or have any
* questions.
*/

// key: compiler.warn.strictfp
// options: -Xlint:strictfp

strictfp class UnneededStrictfpWarning {
}
2 changes: 2 additions & 0 deletions test/langtools/tools/javac/lambda/LambdaTestStrictFPFlag.java
Expand Up @@ -26,6 +26,8 @@
* @bug 8046060
* @summary Different results of floating point multiplication for lambda code block
* @modules jdk.jdeps/com.sun.tools.classfile
* @compile -source 16 LambdaTestStrictFPFlag.java
* @run main LambdaTestStrictFPFlag
*/

import java.io.*;
Expand Down