----- Mail original ----- > De: "John Rose" <john.r.r...@oracle.com> > À: "Rémi Forax" <fo...@univ-mlv.fr> > Cc: "Paul Sandoz" <paul.san...@oracle.com>, "hotspot-dev" > <hotspot-...@openjdk.java.net>, "core-libs-dev" > <core-libs-dev@openjdk.java.net> > Envoyé: Mardi 7 Novembre 2017 21:59:41 > Objet: Re: RFR 8187742 Minimal set of bootstrap methods for dynamic constants
> Good comments! I can handle a couple of them… > > On Nov 7, 2017, at 11:33 AM, Remi Forax <fo...@univ-mlv.fr> wrote: >> >> I fail to see the point of using validateClassAccess(), if the BSM is used >> through an indy, the type is created only if the caller class can create it, >> so >> the test seems redundant. > > That's true, but… > >> If it's not used by an indy, why do we need to test that ? Also, why it's not >> called in invoke ? > > …Enum.valueOf doesn't do a security check; that is its choice. > This means that if you pass it an enum type that is not public > or not in a package exported to you, you can still peek at its > enum values. Meanwhile, when javac emits a reference to > an enum, it does so with getstatic. The getstatic bytecode > *does* perform access checks. The call to validateClassAccess > performs those checks, for alignment with the semantics > of getstatic. The internal use of Enum.valueOf is just a detail > of the emulation of getstatic in the case of an enum. > > (Note to self: Never use enums to implement a shared > secrets pattern.) > > For bootstrap methods I prefer to use the most restrictive > set of applicable access rules, handshaking with the lookup. > > In the case of enums it doesn't matter much, as you say, > because Enum.valueOf leaves the door open. > > I could go either way on this one. As you said, javac use getstatic (the bytecode), why not reuse getstatic (the BSM) in that case, being a field of an enum doesn't seem important and at least you are sure that you have the semantics of getstatic (you miss the cache in java.lang.Class but i'm not sure it worth it). > >> There is also no reason to validate that the parameter type is the right one >> because the VM will validate the return type of the BSM is asTypable to the >> type sent as argument. > > For condy, having the BSM validate types is a code smell > for the reason you mention. Also, when primitives (and value > types) are in the mix, people usually code the validation > incorrectly, since Class.isInstance is the wrong tool, and > the right tool is non-obvious (MHs.identity.asType). > > Are you commenting on the return type adjustment in invoke? > That's just a minor optimization to avoid (un)boxing, which > should have no semantic effect. no, the return type adjustment is ok for me, it avoid unboxing + boxing if the original handle returns a primitive. > >> Some methods use generics, so other don't. Given it's not useful to use >> generics >> if the methods is used as a BSM, i think the generics signature should be >> removed, otherwise, getstatic and invoke should use generics. > > The generics are present to make normal calls from source > code type check. Use cases: Combinators, test code. so getstatic and invoke should have generics in their signature too. > >> to remove the @SuppressWarnings, type should be declared as a >> Class<Class<?>>. > > Paul explained this one to me when I made the same objection. Turns > out that the class literal "Class.class" (which is required to call that > BSM from source code) can only have a raw type. Yuck. So we > declare the formal with a matching amount of raw-ness. yes, right X.class and x.getClass() are the two rules in the JLS that can introduce raw types. I think i prefer 'type' to be typed as a Class<?> in that case, it's not a raw type and you can assign Class<Class>> to it. > >> Why not using getstatic to retrieve the field Type of the wrapper instead ? > > Because the descriptor is a more compact notation for a primitive type. > Primitive type references will be common and we want them to be small. Ok, you have to send the wrapper type if you use getstatic, so you are saving a reference to the wrapper type. > >> If you have invoke(), you do not need enumConstant because you can cook a >> constant method handle Enum.valueOf and send it to invoke. The methods that >> returns VarHandles are trickier because you need a Lookup object. > > The set is minimal, but not that minimal. We *could* express *everything* > using invoke but that would be grotesquely verbose and opaque. > The "extra" API points are thought to be helpful in reducing class file size, > and also clarifying the intentions of the affected constants. > >> getstatic should be renamed to getConstant because this is what it does. > > (I could go either way; since the JVM has ConstantValue attrs, "Constant" > is a Thing. If it's getstatic I prefer getStatic, for alignment with > pre-existing > names in jli.) perhaps getFinalStatic, because it's restricted to final field. > >> wrapping the Throwable in a LinkageError seems wrong to me given that a >> calls to >> a BSM already does that, so getstatic can just declare "throws Throawble" >> like >> invoke and you have the same semantics. > > The point is that we are emulating the semantics of a getstatic instruction, > which only throws LEs. We don't want other stuff to leak out of the > implementation. Remember that "bytecode behavior" is a normative > specification, to use when applicable. It's applicable here. it's not the catch line 155 that bother me, it's the other line 165 (and 162). > > Again, I could go either way on this one. > > Thanks, > — John Rémi
