[cfe-dev] [RFC] Removing libclang APIs to traverse the comment AST

[Dmitri Gribenko]

On Wed, Nov 13, 2013 at 4:51 PM, Sean Silva <silvas at purdue.edu> wrote:
>
> On Wed, Nov 13, 2013 at 1:59 PM, Dmitri Gribenko <gribozavr at gmail.com>
> wrote:
>> The original reason to provide these APIs was to implement a testing
>> mode for comments in c-index-test.  These APIs were not meant for
>> other uses, but I see that I failed to communicate that in the
>> documentation at all.
>
> Why did you even put them in the header *and the exports list* if they
> aren't public API?

As I said, not communicating this is my fault.  I just added these
APIs the same way as we usually add APIs to libclang.

>> > Can you provide a concrete reason for wanting to remove these
>> > APIs?  What actual change is it blocking?
>>
>> I think this is a separate issue, but here are some details.
>>
>> I'll be sending an RFC for this change as well, because I think that
>> this is a major parsing *model* change.  It does not affect the
>> resulting output in normal cases, but will do what the user intended
>> in more unusual cases.
>>
>> On the AST side, it boils down to changing BlockCommandComment to
>> contain multiple paragraphs inside, which will allow commands like
>> \param to have multi-paragraph descriptions.  On libclang side, this
>> change, for example, makes clang_BlockCommandComment_getParagraph()
>> return incomplete results -- just the first paragraph.
>
> It seems like all they can do is get the text of the paragraph, so why not
> just fake up a dummy paragraph whose text content is the content of all the
> paragraphs? and document this restriction and point the user to a new API
> that gives the full access to the paragraphs? I'm also really worried that
> this will also change the XML schema in an incompatible way: the XML schema
> is by extension part of the stability promise of the C API, and can't be
> changed incompatibly without breaking users.

Or we could only return the first paragraph.  Faking a paragraph
requires creating AST nodes, which requires memory allocation on the
ASTContext, which is permanent.

The schema change is:

Index: bindings/xml/comment-xml-schema.rng
===================================================================
--- bindings/xml/comment-xml-schema.rng»(revision 194522)
+++ bindings/xml/comment-xml-schema.rng»(working copy)
@@ -393,7 +393,9 @@
           <!-- In general, template parameters with whitespace discussion
                should not be emitted.  Schema might be more strict here. -->
           <element name="Discussion">
-            <ref name="TextBlockContent" />
+            <oneOrMore>
+              <ref name="TextBlockContent" />
+            </oneOrMore>
           </element>
         </element>
       </oneOrMore>
@@ -435,7 +437,9 @@
                should not be emitted, unless direction is explicitly specified.
                Schema might be more strict here. -->
           <element name="Discussion">
-            <ref name="TextBlockContent" />
+            <oneOrMore>
+              <ref name="TextBlockContent" />
+            </oneOrMore>
           </element>
         </element>
       </oneOrMore>

I think that this change is compatible (but it depends on how one
defines "compatible").  Previously, <Discussion> tag could contain
only a single paragraph; after this change it can contain multiple.

Dmitri

-- 
main(i,j){for(i=2;;i++){for(j=2;j<i;j++){if(!(i%j)){j=0;break;}}if
(j){printf("%d\n",i);}}} /*Dmitri Gribenko <gribozavr at gmail.com>*/