[cfe-commits] [PATCH] TableGen'ize documentation command lists (and break binary compatibility of CXComment)

[Dmitri Gribenko]

On Wed, Sep 5, 2012 at 10:11 AM, Dmitri Gribenko <gribozavr at gmail.com> wrote:
> Hello,
>
> Doug: CCing you since this email is mostly about libclang CXComment
> binary compatibility I need to break for a good reason.
>
> The attached patch TableGen'izes all command lists in
> CommentCommandTraits.cpp and now we have a list of all commands.  This
> is a good thing in itself, but it also enables us to easily implement
> typo correction for command names.
>
> With this change we have objects that contain information about each
> command, so it makes sense to resolve command name just once during
> lexing (currently we store command names as strings and do a linear
> search every time some property value is needed).  Thus comment token
> and AST nodes were changed to contain a command ID -- index into a
> tables of builtin and registered commands.  Unknown commands are
> registered during parsing and thus are also uniformly assigned an ID.
> Using an ID instead of a StringRef is also a nice memory optimization
> since ID is a small integer that fits into a common bitfield in
> Comment class.
>
> This change implies that to get any information about a command (even
> a command name) we need a CommandTraits object to resolve the command
> ID to CommandInfo*.  Currently a fresh temporary CommandTraits object
> is created whenever it is needed since it does not have any state.
> But with this change it has state -- new commands can be registered,
> so I added a CommandTraits object to ASTContext.
>
> Also, in libclang CXComment has to be expanded to include a pointer to
> ASTContext so that all functions working on comment AST nodes can get
> a CommandTraits object.  This breaks binary compatibility of CXComment
> APIs.
>
> We can avoid breaking binary compatibility by storing CommandInfo* in
> AST nodes instead of unsigned command IDs (that need to be resolved).
> But with this approach we won't get that nice memory optimization.
>
> The patch is long, but changes are mostly boring and just change the
> code to use the new APIs to access command properties.  It is the
> architecture and binary compatibility I want to get comments about.

I was reviewing this thread and noticed that I attached the wrong
version of the patch -- sorry for that.  This is the correct one
(still based on the same approach -- CXComment is a value type).

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 --------------
A non-text attachment was scrubbed...
Name: tablegen-command-info-v2.patch
Type: application/octet-stream
Size: 141011 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20120908/99a1ba5a/attachment.obj>