Constant descriptor resolved too soon during constant resolution?

[Brian Goetz]

I've started to work through this example (which came in when I was 
traveling.)  There are a few bugs here.

On 8/2/2018 11:36 AM, Jorn Vernee wrote:
> Hello,
>
> I think I have stumbled upon a bug in the condy-folding branch (tip).
>
> This is the code to reproduce:
>
> ```
> import java.lang.constant.*;
> import static java.lang.constant.ConstantDescs.*;
>
> public class Main {
>
>     static final DirectMethodHandleDesc MHR_CONCAT = MethodHandleDesc.of(
>         DirectMethodHandleDesc.Kind.VIRTUAL,
>         CR_String,
>         "concat",
>         CR_String,
>         CR_String
>     );

Bug #1 -- there is no `putstatic` of MHR_CONCAT in the <clinit> method.  
I understand why we might want to DCE the initialization (all uses of 
the field got constant-propagated away), but now this will fail with 
reflective reads of the variable.  Until we can get permission from the 
JLS to elide unused private statics entirely (which AOT would surely 
love), we need to execute the initializer, and `putstatic` the result 
into the field.  However, that's not your problem here.

You are correct that `d` is not folded here:

     static final ConstantDesc<String> d
         = DynamicConstantDesc.of(BSM_INVOKE, DEFAULT_NAME, CR_Object, 
new ConstantDesc<?>[]{MHR_CONCAT, "Hello, ", "world!"});

and your theory that this is because of the array is almost certainly 
correct.  This (along with the `withArgs()` method) is a sharp edge in 
the API; we deliberately didn't make of(...) a varargs method because of 
overload confusion, but the place where we ended up isn't ideal either.  
So I'll go back and see what we can do there.  The bytecode that is 
generated here is the ordinary, non-folded bytecode, and it works as 
expected, but the API is less than ideal.


Here's where things start to get bad.  You do:

         ConstantDesc<String> dd = 
DynamicConstantDesc.<String>of(BSM_INVOKE).withArgs(MHR_CONCAT, "Hello, 
", "world!");

which gets turned into an LDC:

         12: ldc           #6                  // Dynamic 
#0:_:Ljava/lang/constant/DynamicConstantDesc;

defined as

   0: #42 REF_invokeStatic 
java/lang/constant/DynamicConstantDesc.constantBootstrap:(Ljava/lang/invoke/MethodHandles$Lookup;Ljava/lang/String;Ljava/lang/Class;Ljava/lang/String;Ljava/lang/String;Ljava/lang/String;Ljava/lang/String;Ljava/lang/String;[Ljava/lang/constant/ConstantDesc;)Ljava/lang/constant/DynamicConstantDesc;
     Method arguments:
       #43 Ljava/lang/invoke/ConstantBootstraps;
       #44 invoke
       #45 
(Ljava/lang/invoke/MethodHandles$Lookup;Ljava/lang/String;Ljava/lang/Class;Ljava/lang/invoke/MethodHandle;[Ljava/lang/Object;)Ljava/lang/Object;
       #8 _
       #46 Ljava/lang/Object;
       #47 REF_invokeVirtual 
java/lang/String.concat:(Ljava/lang/String;)Ljava/lang/String;
       #12 Hello,
       #13 world!

This is because the compiler saw that DCD.of(...) was @Foldable, so it 
executed it reflectively, got a Constable out, and asked that Constable 
for its nominal description, as per:

@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();
     System.arraycopy(bootstrapArgs,0, args,5, bootstrapArgs.length);
     return Optional.of(DynamicConstantDesc.of(BSM_DYNAMICCONSTANTDESC, ConstantDescs.DEFAULT_NAME,
                                               CR_DynamicConstantDesc, args));
}

So this should result in a DCD whose BSM is DCD::constantBootstrap, with 
static args:

     String owner = "LConstantBootstraps;"
     String name = "invoke"
     String desc = "(...)"
     String constantName = _
     String constantType = Object
     args = [ MH[String::concat], "Hello", "World" ]

The problem is that the bootstrap is expecting the args in the form of 
ConstantDesc, but what got written to the CP was a live object.  So who 
is to blame here?  It could be describeConstable(), or 
constantBootstrap(), or the compiler. Let's dig.

The DCD starts out in memory being fully nominal, and we're asking for a 
nominal descriptor of the _DCD itself_, because we want to store _it_ in 
the CP.  (This is the stuff that makes your head hurt.)  So what goes 
into the CP should be nominal.  So the constantBootstrap() signature 
looks right to me, and the mistake we have made is that we should 
recursively call describeConstable() on the bootstrap arguments in 
DCD.describeConstable() (and possibly reverse the process in 
constantBootstrap).

So Bug #2 is that DCD.describeConstable (and maybe 
constantBootstrap)needs to quote its arguments before handing them back 
to a would-be Constable-izer (like the compiler.)