[foreign] RFR jextract should model clang Cursors as Trees

[Sundararajan Athijegannathan]

I've made few further changes:

* Removed main method AsmCodeFactory class

* TreePrinter refactored as a separate class and Parser's (test) main is 
now uses it

* Removed CallbackTree (yes, it was from an earlier attempt) and 
associated visitor method

Updated: http://cr.openjdk.java.net/~sundar/8210263/webrev.01

Thanks.
-Sundar

On 31/08/18, 10:26 PM, Maurizio Cimadamore wrote:
> Great work; I think this goes a long way towards putting some more 
> solid foundation under jextract. It also allows, by putting most of 
> the clang dependencies right into the immediate frontend, to make the 
> rest of the code less sensitive to API changes (e.g. should we go from 
> C to C++ clang API). Some comments below (some of those might be 
> overly general and you might want to deal with them in followup changes):
>
> * Thanks for adding the equals/hashcode method in Source[Location, Range]
>
> * While it's probably early now, I think there are a lot of steps 
> which might be translated into their own visitor - e.g. filtering 
> cursors seems one of them (happens between Context and Parser)
>
> * I also believe that, moving forward, classes such as Context and 
> HeaderFile should be all about keeping state, and not about defining 
> behavior (which something that should be delegated to tree visitors)
>
> * I like that AsmCodeFactory just become a visitor!
>
> * Macro handling became a bit better (since we now compute macros on 
> parsing, and then delegate generation to the code factory) - I like 
> that; but let's also keep in mind that macro computation can become 
> its own tree step
>
> * Nit: why is there a main method in ASMCodeFactory?
>
> * It feels like inside Parser there's a tree printer (see main 
> method). And that this printer should probably be unified with the 
> main Printer class. And, pulling on this string some more, I'd like 
> this printer exposed as its own class, so that we can use it to unit 
> test parsing.
>
> * It feels like we could decouple cursors from the tree even more; for 
> instance, names are not reified in the AST (and types, too). This 
> means that it is not possible, for instance, to define a visitor step 
> which replaces some of the names (e.g. something like that would be 
> needed for typedefs). The code, as now, will keep going back to the 
> cursor.
>
> * what exactly is CallbackTree? It seems unused (and so is one of the 
> two createEnum overloads). I believe that is a leftover of some 
> earlier attempts?
>
> P.S.
> Fetched patch, ran all tests, all clear on Linux x64.
>
> Cheers
> Maurizio
>
>
> On 31/08/18 16:13, Sundararajan Athijegannathan wrote:
>> Please review.
>>
>> Bug: https://bugs.openjdk.java.net/browse/JDK-8210263
>> Webrev: http://cr.openjdk.java.net/~sundar/8210263/webrev.00/
>>
>> Thanks,
>> -Sundar
>