Skip to content

Commit

Permalink
8199429: [lworld] value type (re)constructors expressions should not …
Browse files Browse the repository at this point in the history
…be discardable
  • Loading branch information
jespersm committed Oct 10, 2024
1 parent 0ae00ea commit 793f07d
Show file tree
Hide file tree
Showing 9 changed files with 127 additions and 0 deletions.
51 changes: 51 additions & 0 deletions src/java.base/share/classes/java/lang/MustUse.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
/*
* Copyright (c) 2024, 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. Oracle designates this
* particular file as subject to the "Classpath" exception as provided
* by Oracle in the LICENSE file that accompanied this code.
*
* 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.
*/

package java.lang;

import java.lang.annotation.Documented;
import java.lang.annotation.Retention;
import java.lang.annotation.RetentionPolicy;
import java.lang.annotation.Target;

import static java.lang.annotation.ElementType.*;

/**
* A method element annotated {@code @MustUse} is one that returns a value
* which is supposed to be used, since not consuming it will likely be a mistake.
*
* In some cases, the compiler can be certain that the value is not used, and
* in such cases, it may issue an error even in the absense of this annotation.
*
* <p>Compilers issue warnings when a computed value is returned but not assigned
* to a variable or similarly used in different fashion.
*
* @since 24
*/
@Documented
@Retention(RetentionPolicy.RUNTIME)
@Target(value={METHOD})
public @interface MustUse {
}
Original file line number Diff line number Diff line change
Expand Up @@ -338,6 +338,11 @@ public static EnumSet<Flag> asFlagSet(long flags) {
*/
public static final long MIGRATED_VALUE_CLASS = 1L<<57; //ClassSymbols only

/**
* Flag to indicate the given MethodSymbol returns a value which should be used.
*/
public static final long MUST_USE = 1L<<53; //MethodSymbols only

/**
* Flag to indicate the given symbol has a @Deprecated annotation.
*/
Expand Down Expand Up @@ -563,6 +568,7 @@ public String toString() {
MODULE(Flags.MODULE),
AUTOMATIC_MODULE(Flags.AUTOMATIC_MODULE),
SYSTEM_MODULE(Flags.SYSTEM_MODULE),
MUST_USE(Flags.MUST_USE),
DEPRECATED_ANNOTATION(Flags.DEPRECATED_ANNOTATION),
DEPRECATED_REMOVAL(Flags.DEPRECATED_REMOVAL),
HAS_RESOURCE(Flags.HAS_RESOURCE),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -371,6 +371,10 @@ public boolean isDeprecated() {
return (flags_field & DEPRECATED) != 0;
}

public boolean isPrecious() {
return (flags_field & MUST_USE) != 0;
}

public boolean hasDeprecatedAnnotation() {
return (flags_field & DEPRECATED_ANNOTATION) != 0;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -205,6 +205,7 @@ public static Symtab instance(Context context) {
public final Type overrideType;
public final Type retentionType;
public final Type deprecatedType;
public final Type mustUseType;
public final Type suppressWarningsType;
public final Type supplierType;
public final Type inheritedType;
Expand Down Expand Up @@ -602,6 +603,7 @@ public <R, P> R accept(ElementVisitor<R, P> v, P p) {
overrideType = enterClass("java.lang.Override");
retentionType = enterClass("java.lang.annotation.Retention");
deprecatedType = enterClass("java.lang.Deprecated");
mustUseType = enterClass("java.lang.MustUse");
suppressWarningsType = enterClass("java.lang.SuppressWarnings");
supplierType = enterClass("java.util.function.Supplier");
inheritedType = enterClass("java.lang.annotation.Inherited");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -366,6 +366,12 @@ private <T extends Attribute.Compound> void annotateNow(Symbol toAnnotate,
}
}

if (!c.type.isErroneous()
&& toAnnotate.kind == MTH
&& types.isSameType(c.type, syms.mustUseType)) {
toAnnotate.flags_field |= Flags.MUST_USE;
}

if (!c.type.isErroneous()
&& types.isSameType(c.type, syms.previewFeatureType)) {
toAnnotate.flags_field |= Flags.PREVIEW_API;
Expand Down
12 changes: 12 additions & 0 deletions src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Attr.java
Original file line number Diff line number Diff line change
Expand Up @@ -2379,6 +2379,18 @@ public void visitExec(JCExpressionStatement tree) {
//see Infer.instantiatePolymorphicSignatureInstance()
Env<AttrContext> localEnv = env.dup(tree);
attribExpr(tree.expr, localEnv);
// Check if a returned value is a "MustUse"
// TODO: Should likely do more more inference
if (tree.expr instanceof JCMethodInvocation invocation) {
if (invocation.meth instanceof JCFieldAccess select) {
if (select.sym != null && select.sym.kind == MTH) {
if (select.sym.isPrecious()) {
log.error(invocation.meth.pos(), Errors.MethodReturnedMustUseValue(select.selected.type.tsym, select.sym));
}
}
}
}

result = null;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1938,6 +1938,10 @@ compiler.warn.has.been.deprecated.module=\
compiler.warn.has.been.deprecated.for.removal.module=\
module {0} has been deprecated and marked for removal

# 0: symbol, 1: symbol
compiler.err.method.returned.must.use.value=\
The return value from {0}.{1} should be used, or not called

# 0: symbol
compiler.warn.sun.proprietary=\
{0} is internal proprietary API and may be removed in a future release
Expand Down
36 changes: 36 additions & 0 deletions test/langtools/tools/javac/valhalla/must_use/MustUseValueTest.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
/*
* @test /nodynamiccopyright/
* @summary Xxx.
* @enablePreview
*
* @compile/fail/ref=MustUseValueTest.out -XDrawDiagnostics MustUseValueTest.java
*/
public value final class MustUseValueTest implements java.io.Serializable {

public static void main(String[] args) {
var array = new String[] { "A", "B", "C", };

Cursor c = new Cursor(array, 0, 3);
c.next(); // bad code - ERROR
c = c.next(); // good code - NOT ERROR

System.out.println("Should be B: " + c.current());
}

value record Cursor<T>(T[] array, int position, int length) {
Cursor {
if (position < 0 || position >= length) {
throw new IndexOutOfBoundsException("index %d outside array bounds [0;%d[".formatted(position, length));
}
}

T current() {
return array[position];
}

@MustUse
Cursor<T> next() {
return new Cursor(array, position+1, length);
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
MustUseValueTest.java:14:10: compiler.err.method.returned.must.use.value: MustUseValueTest.Cursor, next()
- compiler.note.unchecked.filename: MustUseValueTest.java
- compiler.note.unchecked.recompile
- compiler.note.preview.filename: MustUseValueTest.java, DEFAULT
- compiler.note.preview.recompile
1 error

0 comments on commit 793f07d

Please sign in to comment.