[lworld] RFR: 8279428: [lworld] Revalue ACC_PRIMITIVE to be 0x800 to align with draft JVMS [v2]
Srikanth Adayapalam
sadayapalam at openjdk.java.net
Fri Jan 7 11:25:43 UTC 2022
On Fri, 7 Jan 2022 09:23:36 GMT, Aggelos Biboudis <duke at openjdk.java.net> wrote:
>> But ACC_STRICT and STRICT_FP symbolize/represent the same and carry the same value. (The latter is javac's internal symbol.)
>
> Got it, right!
OK, here is an incremental fix that addresses this concern. Some notes.
- First of all, we cant define PRIMITIVE_CLASS to be 0x800 because this bit pattern is already claimed by STRICT_FP (javac speak for ACC_STRICT which is class file speak)
- ACC_STRICT technically can never be applied to a class in the class file although in javac internal ASTs a class can be marked STRICT_FP to indicate that strictness in FP operations should be propagated to methods of the class. So there is a clash and we choose a different bit encoding.
- A note about naming. I chose PRIMITIVE_CLASS rather than PRIMITIVE to avoid confusion with basic/builtin primitives.
- Now PRIMITIVE_CLASS (javac internal symbol/encoding) needs to be mapped to ACC_PRIMITIVE (JVMS symbol/encoding) at the time of class file writing. This happens in com.sun.tools.javac.jvm.ClassWriter#adjustFlags
- According to JVMS Table 4.1-B. Class access and property modifiers and Table 4.7.6-A. Nested class access and property flags ACC_STRICT is not a valid flag on a class file and so ....
- JDK-7165659 (javac incorrectly sets strictfp access flag on inner-classes) should have arguably cleared STRICT_FP from LocalClassFlags in Flags.java but chose to clear it at the time of class file writing.
So the present update
- Removes STRICT_FP from LocalClassFlags
- Remove PRIMITIVE_CLASS from LocalClassFlags - Class flags are only 16 bits and PRIMITIVE_CLASS is 1 << 16 and so can't really fit in the class file flag and also needs mapping to ACC_PRIMITIVE. It was a bug to include it in the first place.
- Add ACC_PRIMITIVE to LocalClassFlags since that is valid with Valhalla
(Note LocalClassFlags is used as the base to subsequently define many other symbolic constants)
- Update the series of Extended*Flags constants to include PRIMITIVE_CLASS since the latter is really an extended flag (beyond 16 bits)
- Introduce two new Extended*Flags symbols for which no need existed till now.
The one still open minor oddity is why in com.sun.tools.javac.jvm.ClassWriter#writeClassFile we would choose to do
flags = flags & ClassFlags;
while not doing something similar in com.sun.tools.javac.jvm.ClassWriter#writeInnerClasses
but one can convince himself that this is harmless and really a nop. So I have chosen to leave this alone (needs much more ceremony to fix it - we need to define additional constants)
Please take a look and let me know of any more questions
-------------
PR: https://git.openjdk.java.net/valhalla/pull/593
More information about the valhalla-dev
mailing list