[foreign] RFR duplicates in named layouts have to be reported as error

[Maurizio Cimadamore]

To be clear, here's what I'm proposing as a way to limit complexity:

http://cr.openjdk.java.net/~mcimadamore/panama/resolution_context_v2/

I've added a bunch of 'resolveLayout/Function' methods in Util (but they 
could also go in LayoutResolver...) - the binder calls them when needed. 
ABI is now free of unresolved thingies.

Maurizio


On 11/06/18 14:31, Maurizio Cimadamore wrote:
> In hindsight, I believe the very first changeset submitted in this 
> thread was the correct one. Avoiding changes in the binder internal by 
> pushing more complexity onto the parser/unresolved layout API was a 
> siren song, which ultimately leads to a bad complexity ratio for 
> clients of the layout API.
>
> If the binder impl is more complex by having to deal with unresolved 
> layouts explicitly, well, that's not optimal (because it's a source of 
> potential bugs), but it's still way better than having _all_ external 
> clients having to care about resolution context.
>
> Perhaps, one improvement we could concretely make in this area is to 
> have an Util::resolve(Layout) which takes a partial layout and gives 
> you an instantiated layout. That way, we could force resolution in 
> places where we need to - not just on single unresolved nodes, but on 
> entire layout trees.
>
> I think this could help improving code uniformity and also reducing 
> impact of the changes - e.g. with this I think we could avoid sending 
> unresolved layouts into the ABI in the first place?
>
> Maurizio
>
>
> On 05/06/18 09:55, Sundararajan Athijegannathan wrote:
>> As I understand the main change (from what I had earlier) is to use 
>> the most enclosing class as context for layout resolution - 
>> regardless of whether it has NativeHeader annotation or not. Perhaps 
>> the tests can avoid NativeHeader annotation now.
>>
>> For eg. these
>>
>>  test/jdk/java/nicl/abi/sysv/x64/CallingSequenceBuilderTest.java
>>  test/jdk/java/nicl/types/StructTest.java
>>
>>  do not need NativeHeader annotation anymore.
>>
>> Rest is fine.
>>
>> -Sundar
>>
>> On 05/06/18, 2:14 PM, Maurizio Cimadamore wrote:
>>> Sorry - this is it:
>>>
>>> http://cr.openjdk.java.net/~mcimadamore/panama/resolution_context/
>>>
>>> Maurizio
>>>
>>>
>>> On 04/06/18 03:24, Sundararajan Athijegannathan wrote:
>>>>
>>>> Sorry I was on vacation.  The link you provided seems to be the 
>>>> initial named-layout implementation webrev. Will you please provide 
>>>> the correct link?
>>>>
>>>> -Sundar
>>>>
>>>> On 28/05/18, 6:04 PM, Maurizio Cimadamore wrote:
>>>>> Sorry, wrong webrev link, here's the correct one:
>>>>>
>>>>> http://cr.openjdk.java.net/~mcimadamore/panama/layout_resolver/
>>>>>
>>>>> Maurizio
>>>>>
>>>>>
>>>>>
>>>>> On 28/05/18 13:32, Maurizio Cimadamore wrote:
>>>>>> Here's a stab at the current approach; I started with your patch 
>>>>>> and tweaked in places.
>>>>>>
>>>>>> http://cr.openjdk.java.net/~mcimadamore/panama/panama-binder-v3.html
>>>>>>
>>>>>> I think the result is good; the problems you mention are 
>>>>>> resolved, and all use cases are supported.
>>>>>>
>>>>>> Let me know what you think.
>>>>>>
>>>>>> Maurizio
>>>>>>
>>>>>>
>>>>>> On 28/05/18 11:02, Maurizio Cimadamore wrote:
>>>>>>>
>>>>>>>
>>>>>>> On 28/05/18 09:37, Maurizio Cimadamore wrote:
>>>>>>>>
>>>>>>>>
>>>>>>>> On 28/05/18 07:39, Sundararajan Athijegannathan wrote:
>>>>>>>>> Isolated Structs will work if there are no name definitions or 
>>>>>>>>> references in its layout. If there are Unresolved name 
>>>>>>>>> references in its layout, then there has to be a 'context' in 
>>>>>>>>> which the names are defined/resolved. The context cannot be 
>>>>>>>>> entire Java process as it currently is! Two issues with the 
>>>>>>>>> current code: 
>>>>>>>> I agree with all you say. The only thing I'm disagreeing with 
>>>>>>>> is the fact that the context has to be the @NativeHeader 
>>>>>>>> annotation, as that suggests an header-centric nature that was 
>>>>>>>> never intended to be part of the design.
>>>>>>>>
>>>>>>>> Other, more explicit ways to define the context include an 
>>>>>>>> explicit annotation which list all the classes where dependent 
>>>>>>>> layout can be found. I think I'd like something like this 
>>>>>>>> better. Example (names TBD):
>>>>>>>>
>>>>>>>> @NativeStruct("[$(b)](a)")
>>>>>>>> @Friends(B.class)
>>>>>>>> interface A {
>>>>>>>>     B b();
>>>>>>>> }
>>>>>>>>
>>>>>>>> @NativeStruct("[$(a)](b)")
>>>>>>>> @Friends(A.class)
>>>>>>>> interface B {
>>>>>>>>     A a();
>>>>>>>> }
>>>>>>> Now that I have expressed this more explicitly, another idea 
>>>>>>> emerged - what if we assumed that, if no annotation is provided, 
>>>>>>> the context is the outermost class? That will work neutrally 
>>>>>>> with both struct and header interfaces...
>>>>>>>
>>>>>>> Maurizio
>>>>>>>>
>>>>>>>>
>>>>>>>> Maurizio
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>
>>>>>>
>>>>>
>>>
>