hg: valhalla/valhalla: [lworld] Introduce a new annotation java.lang.ValueBased to distinguish migrating value based classes from pure value types and implement leniet semantics for them; Also flip the default for ACC_Flattenable

Tue Apr 3 21:10:42 UTC 2018

Srikanth,

I very much appreciate your introducing this experimental annotation. I do appreciate that you believe this
is not how we will want to handle this in the long-term, and that we want to use javac as an early warning
system to prevent in the field surprise behavior.

I believe we need some experimentation with migration and nullability, to find out if there are benefits
to incremental opt-in vs. immediate behavior changes. So this allows us to do those experiments using
java and javac. If we never had separate compilation, I wouldn’t expect to need this. We will all learn more
by trying concrete examples.

> On Apr 2, 2018, at 8:49 AM, Srikanth <srikanth.adayapalam at oracle.com> wrote:
> 
> 
> 
> On Monday 02 April 2018 05:51 PM, srikanth.adayapalam at oracle.com wrote:
>> Changeset: 32ff82e6b770
>> Author:    sadayapalam
>> Date:      2018-04-02 17:50 +0530
>> URL:       http://hg.openjdk.java.net/valhalla/valhalla/rev/32ff82e6b770
>> 
>> [lworld] Introduce a new annotation java.lang.ValueBased to distinguish migrating value based classes from pure value types and implement leniet semantics for them; Also flip the default for ACC_Flattenable
> 
> Notes:
> 
>     - Introduces a new annotation java.lang.ValueBased with which a __ByValue class may be decorated to indicate that it is a value based class evolving to become a value type.
>     - A __ByValue class without this annotation is considered a "pure" value type.
>     - ACC_FLATTENABLE defaults are flipped. A field whose type is a pure value type will automatically be flagged ACC_FLATTENABLE in its containing instance.
>     - For value based classes evolving to value types, the default flattenable status is NOT flattenable. Request flattening with the field modifier __Flattenable.
Yes - the key point here is that we can experiment with a two-step migration for value-based class. 
Step 1, migrate value-based class to be a value type,
and default continues to be not flattenable and to be nullable. 
Step 2, field container author can opt-in via __Flattenable.
>     - A field whose type is a pure value type can opt out of flattening by using the new field modifier __NotFlattened

Did you mean __NotFlattenable, i.e. nullable? 
My expectation was that at the classfile level, this would not generate ACC_FLATTENABLE. 
>     - Javac will redundantly accept __Flattenable with pure value typed fields and __NotFlattened with VBC typed fields.

>     - @ValueBased instances can be assigned null; null can be cast to them - this would result in a warning at compile time and not an error.

> (the unrealistic code pattern "null instanceof VBC" would also trigger a warning instead of error)
Thank you for making those warnings.
>     - Javac continues to implement a strict mode for pure value types (also see below)
>     - The lenient mode for @ValueBased classes is only for null assignment and (the unrealistic) casting of nulls to @ValueBased types. All identity related violations still result in an error. So does comparison of @ValueBased type instances. (Rationale: Karen originally asked for null related violations to be tolerated on the grounds that the documentation for value based classes does not expressly forbid these. However since == and != are expressly called out as inadvisable operations on value based classes, I have continued to treat them as errors - but I can be convinced to relax this)
My understanding of the model that John worked out was that == followed by null check and .equals pattern would work.
If I understand what you are saying - that reflects what we may see at the bytecode level for backward compatibility.
What you are describing is that javac will not allow identity checks for pure value types or ValueBased classes - because
today that should already be disallowed. We are only testing out leniency for the nullability issues.
So - given the distinction between source and existing bytecode handling - I agree with what you are proposing.

>      - This change set does not identify and annotate the classes in JDK that are value based with @ValueBased. Only introduces the annotation and the new semantics for it.
Thank you. That allows all of us to experiment incrementally.
>     - When an Object instance or a interface instance is cast to a value type, javac will insert a null check IFF the target of the cast is NOT @ValueBased.
>     - When compiling at a source level that is NOT __ByValue aware, if javac encounters a class file that has the ACC_VALUE bit set, the class will be demoted to @ValueBased even though it may not have been annotated with @ValueBased. This will result in null violation warnings to be emitted as appropriate.
Thank you for giving us another way to experiment.

>      - As of this change set the @ValueBased annotation is supposed to always be paired with __ByValue modifier (not checked). A new lint mode in consideration for JDK11 will implement sanity checks for @ValueBased classes that are candidates for evolution at a future date and so are not yet ready to be modified with __ByValue.
Thank you for proposing the JDK11 lint mode.
We are proposing JVM flags for JKDK11 as well - to check identity and nullability assumptions optionally.
I think both of these will help migration.
>     - Javac does not inject null checks with any opcode other than checkcast - see earlier mail raising some questions about the necessity of this with other opcodes.
>     - Expect some surprises in tests due to the flipping of defaults for flattening.
Thank you for the head’s up.
> 
> Karen, please let me know if this adequately covers your proposal.

many thanks,
Karen
> 
> Thanks!
> Srikanth
> 
> 
> 
> 
>> 
>> + src/java.base/share/classes/java/lang/ValueBased.java
>> ! src/java.compiler/share/classes/javax/lang/model/element/Modifier.java
>> ! src/jdk.compiler/share/classes/com/sun/tools/javac/code/Flags.java
>> ! src/jdk.compiler/share/classes/com/sun/tools/javac/code/Symtab.java
>> ! src/jdk.compiler/share/classes/com/sun/tools/javac/code/Types.java
>> ! src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Attr.java
>> ! src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Check.java
>> ! src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Lower.java
>> ! src/jdk.compiler/share/classes/com/sun/tools/javac/jvm/ClassReader.java
>> ! src/jdk.compiler/share/classes/com/sun/tools/javac/parser/JavacParser.java
>> ! src/jdk.compiler/share/classes/com/sun/tools/javac/parser/Tokens.java
>> ! src/jdk.compiler/share/classes/com/sun/tools/javac/resources/compiler.properties
>> ! src/jdk.jshell/share/classes/jdk/jshell/CompletenessAnalyzer.java
>> + test/langtools/tools/javac/diags/examples/BadValueBasedAnno.java
>> + test/langtools/tools/javac/diags/examples/SuspiciousNullMix.java
>> + test/langtools/tools/javac/valhalla/lworld-values/BadValueBased.java
>> + test/langtools/tools/javac/valhalla/lworld-values/BadValueBased.out
>> ! test/langtools/tools/javac/valhalla/lworld-values/CastNullCheckTest.java
>> + test/langtools/tools/javac/valhalla/lworld-values/CheckDefaultFlattenable.java
>> ! test/langtools/tools/javac/valhalla/lworld-values/CheckNullCastable.out
>> + test/langtools/tools/javac/valhalla/lworld-values/DemoteToValueBasedTest.java
>> + test/langtools/tools/javac/valhalla/lworld-values/DemoteToValueBasedTest.out
>> + test/langtools/tools/javac/valhalla/lworld-values/DemoteToValueBasedTest10.out
>> ! test/langtools/tools/javac/valhalla/lworld-values/FlattenableFlagTest.java
>> ! test/langtools/tools/javac/valhalla/lworld-values/Point.java
>> + test/langtools/tools/javac/valhalla/lworld-values/ValueBasedWarningsTest.java
>> + test/langtools/tools/javac/valhalla/lworld-values/ValueBasedWarningsTest.out
>> 
> 