[foreign] RFR: Fix UndefinedLayoutException exception message

[Jorn Vernee]

Maurizio Cimadamore schreef op 2019-01-29 16:46:
> On 29/01/2019 15:19, Jorn Vernee wrote:
>> Maurizio Cimadamore schreef op 2019-01-29 15:59:
>>> Resolving references across interfaces is not expected to work
>>> reliably. This is the primary reason which sends us towards an
>>> extraction model that is library-centric, rather than header-centric,
>>> so that all references in a library should be self-contained.
>> 
>> I've been using a mitigating patch that also scans methods of a Struct 
>> implementation in LayoutResolver when scanning a struct. Under the 
>> assumption that a type's layout is defined on that type itself.
>> 
>>> As for your test, it seems like you can get the same exception simply
>>> by having a single struct interface whose layout has some dandling
>>> unresolved layout (e.g. an unresolved layout whose layout expression
>>> is not defined anywhere)?
>> 
>> The tricky part is triggering the resolution of the layout through the 
>> public API. Trying to allocate a struct will not resolve the struct's 
>> layout. The only place I found that does this is either the test case 
>> I've used, where the dangling layout is in a @NativeFunction 
>> descriptor, or in StructImplGenerator's constructor, which is called 
>> when de referencing a pointer to a struct.
> 
> So, I guess what I'm saying is:
> 
> @NativeStruct("[ ${foo}(foo) ]")
> interface BadStruct extends Struct<BadStruct> {
>     @NativeGetter("foo")
>     Foo foo();
> 
>     interface Foo { }
> }
> 
> Shouldn't this fail when trying to generate the struct implementation?
> (e.g. Scope.allocateStruct(BadStruct.class))

No, that will fail with "UnsupportedOperationException: bitsSize on 
Unresolved"

Scope.allocateStruct(BadStruct.class) will call allocateInternal first 
which throws that UOE, only after that would the implementation be 
generated when calling get() on the returned pointer;

```
private <T> BoundedPointer<T> allocateInternal(LayoutType<T> type, long 
count, int align) {
     ...
     long size = Util.alignUp(type.bytesSize(), align); // !!! calling 
bitSize() on Unresolved throws UOE
     ...
}

@Override
public <T extends Struct<T>> T allocateStruct(Class<T> clazz) {
     // FIXME: is this alignment needed?
     return allocate(LayoutType.ofStruct(clazz), 8).get(); // call to 
'get()' triggers struct generation, and resolution of struct's layout.
}
```

Jorn

> Maurizio
> 
>> 
>> Of course, I could also just use 
>> `LayoutResolver.get(Object.class).resolve(Layout.of("${Undefined}"));`. 
>> Would that be good enough?
>> 
>> Jorn
>> 
>>> Maurizio
>>> 
>>> On 28/01/2019 14:52, Jorn Vernee wrote:
>>>> Ok, it took a while to find a good test case. I was seeing the 
>>>> exception due to a cross-header layout reference, so that's also the 
>>>> test case I added. It seems that that is currently the only 
>>>> realistic use-case where this exception is being thrown. I believe 
>>>> not being able to resolve cross-header layout references is a known 
>>>> issue as well?
>>>> 
>>>> Updated webrev: 
>>>> http://cr.openjdk.java.net/~jvernee/panama/webrevs/fixmessage/webrev.01/ 
>>>> Jorn
>>>> 
>>>> Jorn Vernee schreef op 2019-01-28 14:16:
>>>>> Yes, I will do that. (wasn't sure if it was useful enough).
>>>>> 
>>>>> Jorn
>>>>> 
>>>>> Sundararajan Athijegannathan schreef op 2019-01-28 14:12:
>>>>>> Is it possible to add a test with invalid (unresolvable) name
>>>>>> reference in a layout descriptor and check the exception message?
>>>>>> 
>>>>>> -Sundar
>>>>>> 
>>>>>> On 28/01/19, 6:36 PM, Jorn Vernee wrote:
>>>>>>> Hi,
>>>>>>> 
>>>>>>> JDK-8217245 changed the syntax for Unresolved layouts from 
>>>>>>> `$(Name)` to `${Name}`. This also means Unresolved now has a 
>>>>>>> dedicated `layoutExpression` field instead of relying on the name 
>>>>>>> annotation.
>>>>>>> 
>>>>>>> LayoutResolver.UndefinedLayoutException is still using the name 
>>>>>>> annotation in the exception message, which is now incorrect. This 
>>>>>>> small patch fixes that, and changes it to use 
>>>>>>> Unresolved::layoutExpression().
>>>>>>> 
>>>>>>> Please review.
>>>>>>> 
>>>>>>> Webrev: 
>>>>>>> http://cr.openjdk.java.net/~jvernee/panama/webrevs/fixmessage/webrev.00/ 
>>>>>>> Cheers,
>>>>>>> Jorn