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

[Maurizio Cimadamore]

Looks good - please make sure that it builds on Win before pushing. I 
don't see any weird stuff long-related, but when I added support for 
macro evaluation, I found a couple of warnings on Win which where 
preventing it from building.

Maurizio

On 01/03/2019 13:37, Sundararajan Athijegannathan wrote:
> Thanks for your review!
>
> * added assert after all constructor lookup calls
> * removed unnecessary inlineContentHasTrailingNewline0 indirection
> * "indent" was the intent. oops :)
> * added space after copyright uniformly.
>
> Updated: https://cr.openjdk.java.net/~sundar/8219967/webrev.01/
>
> Thanks,
> -Sundar
>
> On 01/03/19, 5:31 PM, Maurizio Cimadamore wrote:
>>
>> 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