RFR 8187742 Minimal set of bootstrap methods for dynamic constants

[Remi Forax]

Hi Paul,

You have an import static requireNonNull but it is only used once in nullConstant, all other methods use Objects.requireNonNull.

The test that checks that lookup, name and type are null or not is different in each method,
so by example in primitiveClass, if type equals null, you get an IAE. 

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. If it's not used by an indy, why do we need to test that ? Also, why it's not called in invoke ?

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.

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.

In nullConstant, you do not need a requireNonNull here, getting you call isPrimitive() just after.

In primitiveClass, the second test is equivalent to name.length() != 1, to remove the @SuppressWarnings, type should be declared as a Class<Class<?>>. Why not using getstatic to retrieve the field Type of the wrapper instead ?

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. 

getstatic should be renamed to getConstant because this is what it does.
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.

in arrayVarHandle, the doc said that lookup is not used but lookup is used.

in validateClassAccess, the return can be moved at the end of the method.

and as a minor issue, all the ifs should be followed by curly braces per the coding convention.

cheers,
Rémi

----- Mail original -----
> De: "Paul Sandoz" <paul.sandoz at oracle.com>
> À: "core-libs-dev" <core-libs-dev at openjdk.java.net>, "hotspot-dev" <hotspot-dev at openjdk.java.net>
> Envoyé: Mardi 7 Novembre 2017 19:38:57
> Objet: RFR 8187742 Minimal set of bootstrap methods for dynamic constants

> Hi,
> 
> Please review the patch that adds a minimal set of bootstrap methods can be be
> used for producing dynamic constants:
> 
>  http://cr.openjdk.java.net/~psandoz/jdk10/JDK-8187742-condy-bootstraps/webrev/
>  <http://cr.openjdk.java.net/~psandoz/jdk10/JDK-8187742-condy-bootstraps/webrev/>
> 
> The aim is to provide a small but common set of BSMs that are likely useful with
> ldc or as BSM arguments, filling in the gaps for constants that cannot be
> currently represented, such as null, primitive classes and VarHandles. It’s
> possible to get more sophisticated regarding say factories but that is
> something to consider later on.
> 
> This patch is based off the minimal dynamic constant support (still in review):
> 
>  http://mail.openjdk.java.net/pipermail/core-libs-dev/2017-October/049603.html
>  <http://mail.openjdk.java.net/pipermail/core-libs-dev/2017-October/049603.html>
> 
> Paul.