JDK-8270882 : [lworld] Loose ends in reference default primitive class implementation.
  • Type: Bug
  • Component: tools
  • Sub-Component: javac
  • Affected Version: repo-valhalla
  • Priority: P4
  • Status: Resolved
  • Resolution: Not an Issue
  • OS: generic
  • CPU: generic
  • Submitted: 2021-07-19
  • Updated: 2022-02-08
  • Resolved: 2022-02-08
The Version table provides details related to the release that this issue/RFE will be addressed.

Unresolved : Release in which this issue/RFE will be addressed.
Resolved: Release in which this issue/RFE has been resolved.
Fixed : Release in which this issue/RFE has been fixed. The release containing this fix may be available for download as an Early Access Release or a General Availability Release.

To download the current JDK release, click here.
Other
repo-valhallaResolved
Related Reports
Relates :  
Description
This is a follow up to  https://bugs.openjdk.java.net/browse/JDK-8244231 (https://github.com/openjdk/valhalla/pull/482) for issues
deliberately deferred from there:

(1) Types.isSubtype visitor has this code pattern in com.sun.tools.javac.code.Types.TypeRelation#visitClassType

           @Override
            public Boolean visitClassType(ClassType t, Type s) {
                Type sup = asSuper(t, s.tsym);
                if (sup == null) return false;
                // If t is an intersection, sup might not be a class type
                if (!sup.hasTag(CLASS)) return isSubtypeNoCapture(sup, s);
                return sup.tsym == s.tsym
                    && (t.tsym != s.tsym ||
                        (t.isReferenceProjection() == s.isReferenceProjection() && t.isValueProjection() == s.isValueProjection()))
                     // Check type variable containment
                    && (!s.isParameterized() || containsTypeRecursive(s, sup))
                    && isSubtypeNoCapture(sup.getEnclosingType(),
                                          s.getEnclosingType());
            }

The check 
(t.isReferenceProjection() == s.isReferenceProjection() && t.isValueProjection() == s.isValueProjection()))
should be better coded as
t.getFlavor() == s.getFlavor()
but at the moment this is problematic in some cases and needs to be further investigated.

(2) Same issue with isSameTypeVisitor:

return t.tsym == s.tsym
                    && t.isReferenceProjection() == s.isReferenceProjection()
                    && t.isValueProjection() == s.isValueProjection()
                    && visit(getEnclosingType(t), getEnclosingType(s))
                    && containsTypeEquivalent(t.getTypeArguments(), s.getTypeArguments());

should just check for the flavor being the same.

(3) In the definition of com.sun.tools.javac.code.Types#erasure a TODO has been left in with the comment:
// Todo: Handle Type variable case.

(4) There is a TODO task left in LambdaToMethod that needs follow up

// Todo: Investigate to see if a defect should be reported against runtime lambda machinery

(5) There is an half hearted attempt to find a place for ACC_REF_DEFAULT in com.sun.tools.classfile.AccessFlags.
I don't think it belongs there really since it is not a part of the class flags. But I have stuck this in here for now - In javap code to reference ACC_REF_DEFAULT symbolically this is useful (since the compiler's definition from Flags.java is not visible there in javap)


Comments
Much water has flown under the bridge - with the ripping out of the support for reference default primitive classes - these follow up entries have lost relevance. The TODO mentioned in LambdaToMethod got addressed via https://bugs.openjdk.java.net/browse/JDK-8281336 - Closing this tickets as Not an Issue
08-02-2022

A review comment by Maurizio from https://github.com/openjdk/valhalla/pull/482 to be followed up: In com.sun.tools.javac.comp.Attr#checkIdInternal we have this code block: // (a) If symbol is a primitive class and its reference/value projection // is requested via the .ref/.val notation, then adjust the computed type to // reflect this. if (sym.isPrimitiveClass()) { if (sym.isReferenceFavoringPrimitiveClass()) { ... } this code does a bunch of checks - and then creates a new class type with the opposite flavor/polarity. Isn't that what methods like Type::asValueType are supposed to do? E.g. if I have a reference-favoring primitive and I see .val can't I just get the type I want by calling asValueType() on it? If so, should we also have the dual? To be followed up here.
22-07-2021