[foreign-jextract] [Rev 02] RFR: 8239809: Need API to access attributes

[Henry Jen]

On Thu, 27 Feb 2020 23:02:44 GMT, Henry Jen <henryjen at openjdk.org> wrote:

>> I like the new patch - my only reservation is about the withAttribute which accepts a Map which, IMHO exposes to much of the implementation (which is why I was suggesting to add, at least in the public API, just a `withAttribute(String, Constable)` which only makes minimal assumptions on the implementation internals. While this is not a blocker, longer term we will need to decide whether to just allow for adding one attribute at  a time or not - if not, then I think a better way to add multiple attributes at a time would be with a builder:
>> 
>> decl.withAttributes(
>>     a -> a.withAttribute("foo", Foo.class)
>>               .withAttribute("bar", Bar.class)
>> );
>> 
>> But I think the need to go down there has to be justified by concrete use cases. Right now we have only one use case, which is internal (so a public method is not even needed). So perhaps, for the time being allowing to add one attribute at a time is a good enough compromise for the public API (and you can push your Map-accepting method down in DeclarationImpl) ?
>> 
>> I'll leave that to your judgement.
>> 
>> P.S. The TreeMaker changes look great - I like how your approach made the code a lot flatter, well done!
>
> The drawback with the API change is that no specialized subclass override, we can add those but seems too much. Cast if really needed should not be a big issue. If there is an elegant way to do that, I would like to know.

/integrate

-------------

PR: https://git.openjdk.java.net/panama-foreign/pull/33