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

[Sean Silva]

On Wed, Nov 13, 2013 at 9:33 PM, Dmitri Gribenko <gribozavr at gmail.com>wrote:

> On Wed, Nov 13, 2013 at 6:21 PM, Sean Silva <silvas at purdue.edu> wrote:
> >
> > On Wed, Nov 13, 2013 at 9:05 PM, Dmitri Gribenko <gribozavr at gmail.com>
> > wrote:
> >>
> >> On Wed, Nov 13, 2013 at 6:01 PM, Sean Silva <silvas at purdue.edu> wrote:
> >> >
> >> > On Wed, Nov 13, 2013 at 8:25 PM, Dmitri Gribenko <gribozavr at gmail.com
> >
> >> > wrote:
> >> >>
> >> >> On Wed, Nov 13, 2013 at 5:22 PM, Sean Silva <silvas at purdue.edu>
> wrote:
> >> >> >
> >> >> > On Wed, Nov 13, 2013 at 8:08 PM, Dmitri Gribenko
> >> >> > <gribozavr at gmail.com>
> >> >> > wrote:
> >> >> >> 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.
> >> >> >
> >> >> > Why not create it on demand in "freeable" storage?
> >> >>
> >> >> There is no API to free a CXComment node, because it is supposed to
> >> >> represent an real AST node, which is allocated on ASTContext.
> >> >
> >> >
> >> > Can't you allocate it on the CXTranslationUnit (or whatever the
> relevant
> >> > nearby "context" object is that is explicitly created/destroyed)?
> >>
> >> There is no difference, because the lifetime of these contexts is the
> >> same.
> >
> > Wait then what's the big deal with just allocating them permanently? It's
> > not like these comments are going to be gigabytes in size.
>
> Not gigabytes, but possibly very large.  For example, on OS X, the
> text of fully preprocessed Cocoa.h with comments takes over 90 Mb.
>

That's why I was suggesting to create them on-demand; that 90Mb will only
be paid if they happen to call getParagraph on every single comment in
those headers. Also I would suggest to not allocate them in the ASTContext,
and just keep them in the CXTranslationUnit (or whatever the "context"
object is) as strictly a "compatibility" thing.


>
> Anyway, I see that a stable API promise from libclang is not to be
> taken lightly, so it might be the case that we will need to keep these
> APIs.


Breaking APIs that are claimed to be stable and that users depend on is one
of the worst sins in software engineering; far worse than for example
breaking tests (insofar that those tests don't indicate that the stable API
is broken... I think you get what I'm trying to say).
API stability plays roles in business decisions e.g. no budget or developer
time is allocated to "fixing" breakage (since it's stable); if suddenly the
API breaks, then developer time will need to be allocated to fixing it
instead of working on "actual stuff that we planned and that is needed for
the business". Often it will be a nontrivial amount of work to fix API
breakage; even besides developer time. For example the planned deployment
strategy may not admit bundling an XML library, and then suddenly fixing
the problem might require coordination with non-technical management or
marketing ("we had to switch to this other installer software which
requires a 240x240 logo on a dark background instead of 180x180 on a light
background"), etc.; not all breakage is as bad as I just described, but
there are a lot of cases where breakage can cause *much* worse issues than
I just described)


> But this case clearly shows that exposing such low-level
> details is not sustainable, and leads to unfortunate workarounds.
>

Indeed. It is a design decision like any other. Generally speaking any
declaration that is added to a stable public header should be accompanied
by a lot of careful thought about the ramifications.

In this regard, things like (schema-validated) XML are useful since they
generally admit more flexibility in what can be changed without breaking
existing code that processed it.

-- Sean Silva


>
> 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>*/
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-dev/attachments/20131113/f9275a0b/attachment.html>