[foreign] RFR 8219967: Add clang document introspection API support in jdk.internal.clang module

[Maurizio Cimadamore]

Full review:

* cleanup of native code is nice - I like that you enforced foo0 everyehere

* jdk_internal_clang.cpp - missing an assert after constructor lookup, 
at the top

* kudos for implementing the full API - I can see how tedious it was :-)

* Comment.java - why this:

public boolean inlineContentHasTrailingNewline() {
   50         return inlineContentHasTrailingNewline0();
   51     }

E.g. why the 0-version if that's what we want to return in the public 
API too?

* Comment::intent, I think you meant "indent" ?

* Nits: there seems to be a missing newline after copyright in all new 
files but Comment.java (e.g. all the new enums)

Very good piece of work.

Maurizio



On 01/03/2019 11:32, Maurizio Cimadamore wrote:

>
> On 01/03/2019 10:09, Sundararajan Athijegannathan wrote:
>> Please review.
>>
>> Rfe: https://bugs.openjdk.java.net/browse/JDK-8219967
>> Webrev: https://cr.openjdk.java.net/~sundar/8219967/webrev.00/
>>
>> PS. Piggybacking to make few cleanups:
>>
>> Made few native methods in jdk.internal.clang private.
>> Renamed all native methods as foo0 (kind0) pattern as used elsewhere 
>> in JDK rather than foo1.
>>
>> I've not added tests because other parts of jdk.internal.clang don't 
>> seem to have direct tests - those are exercised only via jdk.jextract 
>> tests. For now, I ran Comment class (which has a test main) on few 
>> clang headers with documentation and manually inspected the output.
>
> Quick idea for testing - if we added printing of comment in the cursor 
> (in TreePrinter), then we could have tests which check the 
> well-formedness of the AST (including comments)? I also have added 
> tree evaluation support in there, so it seems like starting to write 
> tree-printer based test could be a good idea (maybe not part of this 
> changeset, but as a followup). Reminds me of what we have done in 
> javac with the DPrinter framework:
>
> http://hg.openjdk.java.net/jdk/jdk/file/tip/test/langtools/tools/javac/lib/DPrinter.java 
>
>
> Maurizio
>
>>
>> Thanks,
>> -Sundar