Constant descriptor resolved too soon during constant resolution?

Jorn Vernee jbvernee at
Tue Aug 14 10:59:58 UTC 2018


I had sent some other general code comments to Brian as well, (I thought 
he was working on it at that moment), but I should probably just have 
sent them here. So here they are again:

These are comments about ``:

- I noticed that in a couple of places the javadoc links to 
`DirectMethodHandleDescImpl`, but I think that should be 
`DirectMethodHandleDesc` instead? (The interface type, not the 
implementation type).

- Also, the javadoc mentions the concept of a "bootstrap method" several 
times, but there's no explanation given of what that is exactly. Maybe 
that could be included in the javadoc for the class? e.g. what 
signatures are valid for bootstrap methods (I recently found out having 
`Lookup, String, Class` as first 3 arguments is not actually required).

- Thirdly, I noticed that when calling `withArgs` you often have to 
manually specify the type parameter. For instance in 

     DynamicConstantDesc.<ConstantDesc<E>>of(BSM_ENUMDESC, CR_EnumDesc)

I've been running into that myself as well when writing descriptor code. 
The fix I have for this in my local copy is to allow `withArgs` to 
return a `DynamicConstantDesc` with a different type parameter. i.e.:

     public <R> DynamicConstantDesc<R> withArgs(ConstantDesc<?>... 
bootstrapArgs) {
         return DynamicConstantDesc.of(bootstrapMethod, constantName, 
constantType, bootstrapArgs);

Adding the type parameter `R` there. This removes the need to manually 
specify the type parameter, and also makes sense semantically since the 
return type of the bootstrap method could depend on one of the arguments 
(e.g. a generic method who's return type is the type of one of it's 

- Lastly, as a suggestion for solving "API sharp edge -- varargs version 
of DCD.of(...)". Instead of providing an overload for each combination 
of arguments you could switch to builder-style methods (which `withArgs` 
already kinda is). i.e. adding the methods `withName(String 
constantName)` and `withType(ClassDesc constantType)`, and then dropping 
the 3 overloads of the `of` method that take a name, type, and name and 
type (but no bootstrapArgs). Keeping the one that takes just a bootstrap 
method descriptor, and the one that takes 4 arguments (but change that 
last one to varargs to allow folding). It's pretty similar, but doesn't 
have the overload confusion (right?).


Additionally, I wanted to ask if you could add some constants to the 
`ConstantDescs` class for working with `LambdaMetafactory`. I have these 
going in my local copy:

diff -r 3b091f0a5df7 
--- a/src/java.base/share/classes/java/lang/constant/ 
Thu Aug 09 22:07:37 2018 +0200
+++ b/src/java.base/share/classes/java/lang/constant/ 
Tue Aug 14 12:35:50 2018 +0200
@@ -35,6 +35,7 @@
  import java.lang.invoke.MethodType;
  import java.lang.invoke.VarHandle;
  import java.lang.invoke.VarHandle.VarHandleDesc;
+import java.lang.invoke.LambdaMetafactory;
  import java.util.Collection;
  import java.util.List;
  import java.util.Map;
@@ -150,6 +151,10 @@
      public static final ClassDesc CR_CallSite = 

+    /** {@link ClassDesc} representing {@link LambdaMetafactory} */
+    @Foldable
+    public static final ClassDesc CR_LambdaMetafactory = 
      /** {@link ClassDesc} representing {@link Collection} */
      public static final ClassDesc CR_Collection = 
@@ -264,6 +269,12 @@
              = ofConstantBootstrap(CR_ConstantBootstraps, "invoke",
                                    CR_Object, CR_MethodHandle, 

+    /** {@link MethodHandleDesc} representing {@link 
LambdaMetafactory#metafactory(MethodHandles.Lookup, String, Class, 
MethodType, MethodHandle, MethodType)} */
+    @Foldable
+    public static final DirectMethodHandleDesc BSM_METAFACTORY_CONDY
+            = ofConstantBootstrap(CR_LambdaMetafactory, "metafactory",
+                                  CR_Object, CR_MethodType, 
CR_MethodHandle, CR_MethodType);
      /** {@link ClassDesc} representing the primitive type {@code int} 
      public static final ClassDesc CR_int = ClassDesc.ofDescriptor("I");
@@ -418,4 +429,36 @@
ClassDesc... paramTypes) {
          return MethodHandleDesc.of(STATIC, clazz, name, 
MethodTypeDesc.of(returnType, paramTypes).insertParameterTypes(0, 
+    /**
+     * Return a {@link DynamicConstantDesc} which resolves to an 
instance of a
+     * functional interface type, as if through an invocation of
+     * {@link LambdaMetafactory#metafactory(MethodHandles.Lookup, 
String, Class, MethodType, MethodHandle, MethodType)}
+     *
+     * @param <T> the type of the functional interface
+     * @param invokedName The name of the method to implement.
+     * @param functionalInterface The functional interface type.
+     * @param samMethodType Signature and return type of method to be 
+     *                      by the function object.
+     * @param implMethod A direct method handle describing the 
+     *                   method which should be called (with suitable 
+     *                   of argument types, return types, and with 
+     *                   arguments prepended to the invocation 
arguments) at
+     *                   invocation time.
+     * @param instantiatedMethodType The signature and return type that 
+     *                               be enforced dynamically at 
invocation time.
+     *                               This may be the same as {@code 
+     *                               or may be a specialization of it.
+     * @return a descriptor that resolves to an instance of the 
functional interface type.
+     */
+    @Foldable
+    public static <T> DynamicConstantDesc<T> 
ofLambdaCondyBootstrap(String invokedName, ClassDesc 
ConstantDesc<MethodType> samMethodType,
ConstantDesc<MethodHandle> implMethod,
ConstantDesc<MethodType> instantiatedMethodType) {
+        // Requiring ConstantDesc<...>s here instead of more specific 
types since arguments might be
+        // computed at link-time.
+        return DynamicConstantDesc.<T>of(BSM_METAFACTORY_CONDY, 
invokedName, functionalInterface)
+                                  .withArgs(samMethodType, implMethod, 
+    }

(Note that some of the javadoc for that last helper method was taken 
from the javadoc in `LambdaMetafactory`. I don't know if that matters)

Since I don't have access to `@Foldable` in user code, I found it 
impossible to use  my own bag-of-constants class, custom descriptor 
types or other helper methods, since they would inhibit folding. I think 
it's a good move making it an internal annotation though, some weird 
problems can occur when using it (e.g. because you're depending on the 
compiler's class path), but that also means that currently any helper 
method, global constant or descriptor type that someone might need has 
to be a part of the standard API.

Best regards,
Jorn Vernee

Vicente Romero schreef op 2018-08-14 02:23:
> On 08/10/2018 04:25 PM, Brian Goetz wrote:
>> Here's my version, similar but with a different error handling 
>> strategy:
>> @Override public Optional<?extends ConstantDesc<ConstantDesc<T>>> 
>> describeConstable() {
>>     ConstantDesc<?>[] args =new ConstantDesc<?>[bootstrapArgs.length 
>> +5];
>>     args[0] =bootstrapMethod.owner().descriptorString();
>>     args[1] =bootstrapMethod.methodName();
>>     args[2] =bootstrapMethod.methodType().descriptorString();
>>     args[3] =constantName;
>>     args[4] =constantType.descriptorString();
>>     for (int i=0; i<bootstrapArgs.length; i++)
>>         args[i+5] = (ConstantDesc<?>) 
>> ((Constable)bootstrapArgs[i]).describeConstable().orElseThrow();
>>     return Optional.of(DynamicConstantDesc.of(BSM_DYNAMICCONSTANTDESC, 
>> ConstantDescs.DEFAULT_NAME,
>> CR_DynamicConstantDesc, args));
>> }
>> All that said, I'm not sure that we land in the right place here; it 
>> is probably more work to LDC this DCD from the constant pool than to 
>> construct a fresh one.  Its possible we want some more control knobs 
>> in Constable to feed back hints into the compiler as to whether or not 
>> constantizing is likely to be profitable, or perhaps we need to 
>> revisit the constantBoostrap() protocol for DCD.
>> So, excellent bug report -- four issues in one!
> yep great bug report and analysis too! getting into this after a time 
> out
>>  - Bug -- compiler incorrectly optimizes away static initializer
>>  - Bug -- DynamicConstantDesc.describeConstable() needs to quote its 
>> static args
>>  - API sharp edge -- varargs version of DCD.of(...)
>>  - Possible API insufficiency -- constant-izing DCD may be an 
>> anti-optimization
> I have pushed fixes to the condy-folding branch for the bugs listed 
> above
> Thanks,
> Vicente

More information about the amber-dev mailing list