[foreign] RFR: Fix UndefinedLayoutException exception message

[Maurizio Cimadamore]

On 29/01/2019 15:57, Jorn Vernee wrote:
> 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"

Then try with

scope.allocate(LayoutType.ofStruct(BadStruct.class)).get().foo()

Maurizio

>
> 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