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

[Henry Jen]

On Thu, 27 Feb 2020 11:06:34 GMT, Maurizio Cimadamore <mcimadamore at openjdk.org> wrote:

>> The pull request has been updated with 1 additional commit.
>
> 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.

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

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