RFR: 7903605: Simplify TypeMaker

Maurizio Cimadamore mcimadamore at openjdk.org
Mon Dec 11 10:39:46 UTC 2023


On Fri, 8 Dec 2023 23:37:01 GMT, Jorn Vernee <jvernee at openjdk.org> wrote:

>> The main goal of this PR is to make TypeMaker simpler, and remove the convoluted logic which stashes pointers, and asks client to call a method to resolve their types.
>> 
>> Instead, this patch tweaks the behavior of the TypeMaker so that when a pointer to a declared type is created, it points to a special supplier. This supplier, when called, looks in the declaration cache for a matching declaration (in the `TreeMaker`). If a corresponding declaration is found, a new declared type pointing to that declaration is returned as a pointee type.
>> 
>> It is also possible for a pointer to point to an opaque struct, in which case there might be no symbol in the declaration cache. In such case we make the supplier return an erroneous type instead.
>> 
>> The patch also addresses a number of related issues:
>> 
>> * Since declarations are stable identity-wise but types are not, I removed the `Attributed` interface and corresponding implementation class. Now all the methods to deal with attributes are in `Declaration` - declarations are the only entities that can be reliably attributed (because of de-duplication).
>> 
>> * I added a mechanism to create "nested" `TreeMaker` - that is, a `TreeMaker` that can delegate a symbol lookup to a parent `TreeMaker`. This is handy when dealing with macros, where we do not want to pollute the original environment, but we still want to be able to resolve declarations included in that environment.
>> 
>> * I have fixed the equals methods in Declaration - these methods ignored attributes and were not working correctly. As a result, I also did some changes in duplicate filter, as using declaration equality does not work (because attributes might mismatch thanks to the added `Skip`).
>
> src/main/java/org/openjdk/jextract/impl/TreeMaker.java line 281:
> 
>> 279:                         // process struct recursively
>> 280:                         pendingFields.add(recordDeclaration(parent, fc).tree());
>> 281:                     } else if (!fc.isBitField() && !fc.spelling().isEmpty()) {
> 
> Spurious change? Or was this condition needed after all?

This (and other changes) have come back as a result of a bad merge I will fix

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

PR Review Comment: https://git.openjdk.org/jextract/pull/155#discussion_r1422275748


More information about the jextract-dev mailing list