hg: valhalla/valhalla: [lworld] Withdraw support for the __Flattenable and __NotFlattened field modifiers.
Srikanth
srikanth.adayapalam at oracle.com
Tue Jun 26 11:23:19 UTC 2018
The attached patch would cause the flattenability modifiers to be
accepted with their erstwhile syntax and semantics when javac is invoked
with -XDallowFlattenabilityModifiers
I'll wait to hear from Karen and push/discard accordingly.
(I am likely away Thursday/Friday, I will be available on Wednesday.
Posting a patch here, so if it is required in my absence it can be
availed. It passes all langtools tests)
Thanks!
Srikanth
On Tuesday 26 June 2018 03:38 PM, Srikanth wrote:
>
>
> On Tuesday 26 June 2018 03:32 PM, Tobias Hartmann wrote:
>> Hi Srikanth,
>>
>> On 26.06.2018 11:37, Srikanth wrote:
>>> I did have an express go from Karen to "nuke" these source level
>>> modifiers before proceeding. But in
>>> light of what you say, I'll take a look and see what is involved in
>>> pushing these under an option
>>> instead.
>> Thanks for looking into it but we should probably wait for Karen to
>> comment on that. I guess I
>> missed the discussion/decision of fully removing these modifiers (I
>> thought we just don't want to
>> support them for LW1).
>
> Sounds good to me. I'll keep a patch ready in any case and after
> hearing from Karen decide to push or discard.
>
> I believe the consensus in the VM is to move away fromACC_FLATTENABLE
> on fields to using the ValueTypes attribute.
>
> Your concern below about the code rot is valid.
>
> Srikanth
>>
>>> Even with that many of the tests would need "massaging" but I guess
>>> it won't shut you out from
>>> retaining the support for non-flattened value type fields.
>> Yes, I'm fine with modifying the tests, I'm just concerned that
>> without testing the implementation
>> of non-flattenable fields in the JVM, the code will start to rot very
>> soon.
>>
>> Thanks,
>> Tobias
>
-------------- next part --------------
diff -r 8b2ca4fdb101 src/java.compiler/share/classes/javax/lang/model/element/Modifier.java
--- a/src/java.compiler/share/classes/javax/lang/model/element/Modifier.java Tue Jun 26 13:02:54 2018 +0530
+++ b/src/java.compiler/share/classes/javax/lang/model/element/Modifier.java Tue Jun 26 16:49:37 2018 +0530
@@ -63,6 +63,17 @@
* @since 1.11
*/
VALUE,
+ /**
+ * The modifier {@code __Flattenable}
+ * @since 1.11
+ */
+ FLATTENABLE,
+
+ /**
+ * The modifier {@code __NotFlattened}
+ * @since 1.11
+ */
+ NOT_FLATTENED,
/** The modifier {@code static} */ STATIC,
/** The modifier {@code final} */ FINAL,
diff -r 8b2ca4fdb101 src/jdk.compiler/share/classes/com/sun/tools/javac/code/Flags.java
--- a/src/jdk.compiler/share/classes/com/sun/tools/javac/code/Flags.java Tue Jun 26 13:02:54 2018 +0530
+++ b/src/jdk.compiler/share/classes/com/sun/tools/javac/code/Flags.java Tue Jun 26 16:49:37 2018 +0530
@@ -230,6 +230,11 @@
public static final long UNION = 1L<<39;
/**
+ * Flag that marks field that is a value instance that can be inlined in the layout.
+ */
+ public static final long FLATTENABLE = 1L<<40;
+
+ /**
* Flag that marks an 'effectively final' local variable.
*/
public static final long EFFECTIVELY_FINAL = 1L<<41;
@@ -321,6 +326,11 @@
public static final long ANONCONSTR_BASED = 1L<<57;
/**
+ * Flag that marks field that is a value instance that SHOULD NOT be inlined in the layout.
+ */
+ public static final long NOT_FLATTENED = 1L<<58;
+
+ /**
* Flag that marks finalize block as body-only, should not be copied into catch clauses.
* Used to implement try-with-resources.
*/
@@ -339,13 +349,13 @@
MethodFlags = AccessFlags | ABSTRACT | STATIC | NATIVE |
SYNCHRONIZED | FINAL | STRICTFP;
public static final long
- ExtendedStandardFlags = (long)StandardFlags | DEFAULT | VALUE,
- ModifierFlags = ((long)StandardFlags & ~INTERFACE) | DEFAULT,
+ ExtendedStandardFlags = (long)StandardFlags | DEFAULT | VALUE | FLATTENABLE | NOT_FLATTENED,
+ ModifierFlags = ((long)StandardFlags & ~INTERFACE) | DEFAULT | FLATTENABLE | NOT_FLATTENED,
InterfaceMethodMask = ABSTRACT | PRIVATE | STATIC | PUBLIC | STRICTFP | DEFAULT,
AnnotationTypeElementMask = ABSTRACT | PUBLIC,
LocalVarFlags = FINAL | PARAMETER,
VarFlags = AccessFlags | FINAL | STATIC |
- VOLATILE | TRANSIENT | ENUM,
+ VOLATILE | TRANSIENT | FLATTENABLE | NOT_FLATTENED | ENUM,
ReceiverParamFlags = PARAMETER;
@@ -360,6 +370,8 @@
if (0 != (flags & STATIC)) modifiers.add(Modifier.STATIC);
if (0 != (flags & FINAL)) modifiers.add(Modifier.FINAL);
if (0 != (flags & TRANSIENT)) modifiers.add(Modifier.TRANSIENT);
+ if (0 != (flags & FLATTENABLE)) modifiers.add(Modifier.FLATTENABLE);
+ if (0 != (flags & NOT_FLATTENED)) modifiers.add(Modifier.NOT_FLATTENED);
if (0 != (flags & VOLATILE)) modifiers.add(Modifier.VOLATILE);
if (0 != (flags & SYNCHRONIZED))
modifiers.add(Modifier.SYNCHRONIZED);
@@ -428,6 +440,8 @@
HYPOTHETICAL(Flags.HYPOTHETICAL),
PROPRIETARY(Flags.PROPRIETARY),
UNION(Flags.UNION),
+ FLATTENABLE(Flags.FLATTENABLE),
+ NOT_FLATTENED(Flags.NOT_FLATTENED),
EFFECTIVELY_FINAL(Flags.EFFECTIVELY_FINAL),
CLASH(Flags.CLASH),
AUXILIARY(Flags.AUXILIARY),
diff -r 8b2ca4fdb101 src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Attr.java
--- a/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Attr.java Tue Jun 26 13:02:54 2018 +0530
+++ b/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Attr.java Tue Jun 26 16:49:37 2018 +0530
@@ -1201,6 +1201,10 @@
setSyntheticVariableType(tree, v.type);
}
}
+ if (v.owner.kind == TYP && types.isValue(v.type) && !types.isValueBased(v.type)) {
+ if ((v.flags() & (STATIC | NOT_FLATTENED)) == 0)
+ v.flags_field |= FLATTENABLE;
+ }
result = tree.type = v.type;
}
finally {
diff -r 8b2ca4fdb101 src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Check.java
--- a/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Check.java Tue Jun 26 13:02:54 2018 +0530
+++ b/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Check.java Tue Jun 26 16:49:37 2018 +0530
@@ -93,6 +93,7 @@
private final Target target;
private final Profile profile;
private final boolean warnOnAnyAccessToMembers;
+ private final boolean rejectValueMembershipCycles;
private final boolean allowGenericsOverValues;
private final boolean allowValueBasedClasses;
@@ -135,6 +136,7 @@
source = Source.instance(context);
target = Target.instance(context);
warnOnAnyAccessToMembers = options.isSet("warnOnAccessToMembers");
+ rejectValueMembershipCycles = options.isSet("rejectValueMembershipCycles");
allowGenericsOverValues = options.isSet("allowGenericsOverValues");
allowValueBasedClasses = options.isSet("allowValueBasedClasses");
Target target = Target.instance(context);
@@ -2226,7 +2228,11 @@
}
// where
private boolean cyclePossible(VarSymbol symbol) {
- return !symbol.isStatic() && types.isValue(symbol.type);
+ if (symbol.isStatic())
+ return false;
+ if (rejectValueMembershipCycles && types.isValue(symbol.type))
+ return true;
+ return (symbol.flags() & FLATTENABLE) != 0 && types.isValue(symbol.type);
}
void checkNonCyclicDecl(JCClassDecl tree) {
diff -r 8b2ca4fdb101 src/jdk.compiler/share/classes/com/sun/tools/javac/jvm/ClassReader.java
--- a/src/jdk.compiler/share/classes/com/sun/tools/javac/jvm/ClassReader.java Tue Jun 26 13:02:54 2018 +0530
+++ b/src/jdk.compiler/share/classes/com/sun/tools/javac/jvm/ClassReader.java Tue Jun 26 16:49:37 2018 +0530
@@ -2932,6 +2932,11 @@
***********************************************************************/
long adjustFieldFlags(long flags) {
+ if ((flags & ACC_FLATTENABLE) != 0) {
+ flags &= ~ACC_FLATTENABLE;
+ if (allowValueTypes)
+ flags |= FLATTENABLE;
+ }
return flags;
}
diff -r 8b2ca4fdb101 src/jdk.compiler/share/classes/com/sun/tools/javac/jvm/ClassWriter.java
--- a/src/jdk.compiler/share/classes/com/sun/tools/javac/jvm/ClassWriter.java Tue Jun 26 13:02:54 2018 +0530
+++ b/src/jdk.compiler/share/classes/com/sun/tools/javac/jvm/ClassWriter.java Tue Jun 26 16:49:37 2018 +0530
@@ -1179,8 +1179,6 @@
*/
void writeField(VarSymbol v) {
int flags = adjustFlags(v.flags());
- if ((v.flags() & STATIC) == 0 && types.isValue(v.type))
- flags |= ACC_FLATTENABLE;
databuf.appendChar(flags);
if (dumpFieldModifiers) {
PrintWriter pw = log.getWriter(Log.WriterKind.ERROR);
@@ -1931,6 +1929,8 @@
result &= ~ABSTRACT;
if ((flags & VALUE) != 0)
result |= ACC_VALUE;
+ if ((flags & FLATTENABLE) != 0)
+ result |= ACC_FLATTENABLE;
return result;
}
diff -r 8b2ca4fdb101 src/jdk.compiler/share/classes/com/sun/tools/javac/parser/JavacParser.java
--- a/src/jdk.compiler/share/classes/com/sun/tools/javac/parser/JavacParser.java Tue Jun 26 13:02:54 2018 +0530
+++ b/src/jdk.compiler/share/classes/com/sun/tools/javac/parser/JavacParser.java Tue Jun 26 16:49:37 2018 +0530
@@ -181,6 +181,7 @@
this.keepLineMap = keepLineMap;
this.errorTree = F.Erroneous();
endPosTable = newEndPosTable(keepEndPositions);
+ this.allowFlattenabilityModifiers = fac.options.isSet("allowFlattenabilityModifiers");
}
protected AbstractEndPosTable newEndPosTable(boolean keepEndPositions) {
@@ -197,6 +198,10 @@
*/
boolean allowStringFolding;
+ /** Switch: should we allow value type field flattenability modifiers?
+ */
+ boolean allowFlattenabilityModifiers;
+
/** Switch: should we keep docComments?
*/
boolean keepDocComments;
@@ -320,6 +325,8 @@
case PROTECTED:
case STATIC:
case TRANSIENT:
+ case FLATTENABLE:
+ case NOTFLATTENED:
case NATIVE:
case VOLATILE:
case SYNCHRONIZED:
@@ -2882,6 +2889,8 @@
case PUBLIC : flag = Flags.PUBLIC; break;
case STATIC : flag = Flags.STATIC; break;
case TRANSIENT : flag = Flags.TRANSIENT; break;
+ case FLATTENABLE : checkSourceLevel(Feature.VALUE_TYPES); checkFlattenabilitySpecOk(token.pos); flag = Flags.FLATTENABLE; break;
+ case NOTFLATTENED: checkSourceLevel(Feature.VALUE_TYPES); checkFlattenabilitySpecOk(token.pos); flag = Flags.NOT_FLATTENED; break;
case FINAL : flag = Flags.FINAL; break;
case ABSTRACT : flag = Flags.ABSTRACT; break;
case NATIVE : flag = Flags.NATIVE; break;
@@ -2930,6 +2939,12 @@
storeEnd(mods, S.prevToken().endPos);
return mods;
}
+ // where
+ private void checkFlattenabilitySpecOk(int pos) {
+ if (!allowFlattenabilityModifiers) {
+ log.error(pos, Errors.FlattenabilityModifiersDisallowed);
+ }
+ }
/** Annotation = "@" Qualident [ "(" AnnotationFieldValues ")" ]
*
diff -r 8b2ca4fdb101 src/jdk.compiler/share/classes/com/sun/tools/javac/parser/Tokens.java
--- a/src/jdk.compiler/share/classes/com/sun/tools/javac/parser/Tokens.java Tue Jun 26 13:02:54 2018 +0530
+++ b/src/jdk.compiler/share/classes/com/sun/tools/javac/parser/Tokens.java Tue Jun 26 16:49:37 2018 +0530
@@ -162,6 +162,8 @@
THROW("throw"),
THROWS("throws"),
TRANSIENT("transient"),
+ FLATTENABLE("__Flattenable"),
+ NOTFLATTENED("__NotFlattened"),
TRY("try"),
VALUE("__ByValue"),
VDEFAULT("__MakeDefault"),
diff -r 8b2ca4fdb101 src/jdk.compiler/share/classes/com/sun/tools/javac/resources/compiler.properties
--- a/src/jdk.compiler/share/classes/com/sun/tools/javac/resources/compiler.properties Tue Jun 26 13:02:54 2018 +0530
+++ b/src/jdk.compiler/share/classes/com/sun/tools/javac/resources/compiler.properties Tue Jun 26 16:49:37 2018 +0530
@@ -3435,3 +3435,6 @@
# 0: type
compiler.warn.potential.null.pollution=\
Potential null pollution from nullable type {0}
+
+compiler.err.flattenability.modifiers.disallowed=\
+ This modifier is allowed only with -XDallowFlattenabilityModifiers
diff -r 8b2ca4fdb101 src/jdk.jshell/share/classes/jdk/jshell/CompletenessAnalyzer.java
--- a/src/jdk.jshell/share/classes/jdk/jshell/CompletenessAnalyzer.java Tue Jun 26 13:02:54 2018 +0530
+++ b/src/jdk.jshell/share/classes/jdk/jshell/CompletenessAnalyzer.java Tue Jun 26 16:49:37 2018 +0530
@@ -223,6 +223,8 @@
PROTECTED(TokenKind.PROTECTED, XDECL1), // protected
PUBLIC(TokenKind.PUBLIC, XDECL1), // public
TRANSIENT(TokenKind.TRANSIENT, XDECL1), // transient
+ FLATTENABLE(TokenKind.FLATTENABLE, XDECL1), // __Flattenable
+ NOTFLATTENED(TokenKind.NOTFLATTENED, XDECL1), // __NotFlattened
VOLATILE(TokenKind.VOLATILE, XDECL1), // volatile
// Declarations and type parameters (thus expressions)
diff -r 8b2ca4fdb101 test/langtools/tools/javac/diags/examples.not-yet.txt
--- a/test/langtools/tools/javac/diags/examples.not-yet.txt Tue Jun 26 13:02:54 2018 +0530
+++ b/test/langtools/tools/javac/diags/examples.not-yet.txt Tue Jun 26 16:49:37 2018 +0530
@@ -200,3 +200,4 @@
compiler.err.make.default.with.nonvalue
compiler.err.value.may.not.extend
compiler.warn.potential.null.pollution
+compiler.err.flattenability.modifiers.disallowed
diff -r 8b2ca4fdb101 test/langtools/tools/javac/valhalla/lworld-values/CheckDefaultFlattenable.java
--- a/test/langtools/tools/javac/valhalla/lworld-values/CheckDefaultFlattenable.java Tue Jun 26 13:02:54 2018 +0530
+++ b/test/langtools/tools/javac/valhalla/lworld-values/CheckDefaultFlattenable.java Tue Jun 26 16:49:37 2018 +0530
@@ -27,7 +27,7 @@
* @test
* @summary Check default setting of ACC_FLATTENABLE
* @modules jdk.jdeps/com.sun.tools.classfile
- * @compile -XDallowValueBasedClasses CheckDefaultFlattenable.java
+ * @compile -XDallowValueBasedClasses -XDallowFlattenabilityModifiers CheckDefaultFlattenable.java
* @run main CheckDefaultFlattenable
*/
@@ -44,12 +44,12 @@
}
X x1;
- static X x2;
- X x3;
+ __NotFlattened X x2;
+ __Flattenable X x3;
Y y1;
- static Y y2;
- Y y3;
+ __NotFlattened Y y2;
+ __Flattenable Y y3;
public static void main(String[] args) throws Exception {
ClassFile cls = ClassFile.read(CheckDefaultFlattenable.class.getResourceAsStream("CheckDefaultFlattenable.class"));
@@ -73,8 +73,8 @@
}
if (field.getName(cls.constant_pool).equals("y1")) {
checks ++;
- if ((field.access_flags.flags & AccessFlags.ACC_FLATTENABLE) == 0)
- throw new AssertionError("Expected flattenable flag missing");
+ if ((field.access_flags.flags & AccessFlags.ACC_FLATTENABLE) != 0)
+ throw new AssertionError("Unexpected flattenable flag found");
}
if (field.getName(cls.constant_pool).equals("y2")) {
checks ++;
diff -r 8b2ca4fdb101 test/langtools/tools/javac/valhalla/lworld-values/CheckFlattenableCycles.java
--- a/test/langtools/tools/javac/valhalla/lworld-values/CheckFlattenableCycles.java Tue Jun 26 13:02:54 2018 +0530
+++ b/test/langtools/tools/javac/valhalla/lworld-values/CheckFlattenableCycles.java Tue Jun 26 16:49:37 2018 +0530
@@ -2,7 +2,7 @@
* @test /nodynamiccopyright/
* @summary Check for cycles through fields declared flattenable.
*
- * @compile/fail/ref=CheckFlattenableCycles.out -XDrawDiagnostics CheckFlattenableCycles.java
+ * @compile/fail/ref=CheckFlattenableCycles.out -XDrawDiagnostics -XDallowFlattenabilityModifiers CheckFlattenableCycles.java
*/
final __ByValue class CheckFlattenableCycles {
@@ -10,11 +10,11 @@
CheckFlattenableCycles cfc;
}
__ByValue final class InnerValue {
- final CheckFlattenableCycles cfc = __MakeDefault CheckFlattenableCycles(); // Error.
+ final __Flattenable CheckFlattenableCycles cfc = __MakeDefault CheckFlattenableCycles(); // Error.
}
- final CheckFlattenableCycles cfc = __MakeDefault CheckFlattenableCycles(); // Error.
+ final __Flattenable CheckFlattenableCycles cfc = __MakeDefault CheckFlattenableCycles(); // Error.
final int i = 10;
final String s = "blah";
final InnerRef ir = new InnerRef(); // OK.
- final InnerValue iv = __MakeDefault InnerValue();
-}
+ final __Flattenable InnerValue iv = __MakeDefault InnerValue();
+}
\ No newline at end of file
diff -r 8b2ca4fdb101 test/langtools/tools/javac/valhalla/lworld-values/CheckFlattenableCycles.out
--- a/test/langtools/tools/javac/valhalla/lworld-values/CheckFlattenableCycles.out Tue Jun 26 13:02:54 2018 +0530
+++ b/test/langtools/tools/javac/valhalla/lworld-values/CheckFlattenableCycles.out Tue Jun 26 16:49:37 2018 +0530
@@ -1,3 +1,3 @@
-CheckFlattenableCycles.java:15:34: compiler.err.cyclic.value.type.membership: CheckFlattenableCycles
-CheckFlattenableCycles.java:13:38: compiler.err.cyclic.value.type.membership: CheckFlattenableCycles.InnerValue
-2 errors
+CheckFlattenableCycles.java:15:48: compiler.err.cyclic.value.type.membership: CheckFlattenableCycles
+CheckFlattenableCycles.java:13:52: compiler.err.cyclic.value.type.membership: CheckFlattenableCycles.InnerValue
+2 errors
\ No newline at end of file
More information about the valhalla-dev
mailing list