<br><br><div class="gmail_quote">On Wed, Sep 5, 2012 at 9:11 AM, Dmitri Gribenko <span dir="ltr"><<a href="mailto:gribozavr@gmail.com" target="_blank">gribozavr@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
Hello,<br>
<br>
Doug: CCing you since this email is mostly about libclang CXComment<br>
binary compatibility I need to break for a good reason.<br>
<br>
The attached patch TableGen'izes all command lists in<br>
CommentCommandTraits.cpp and now we have a list of all commands.  This<br>
is a good thing in itself, but it also enables us to easily implement<br>
typo correction for command names.<br>
<br>
With this change we have objects that contain information about each<br>
command, so it makes sense to resolve command name just once during<br>
lexing (currently we store command names as strings and do a linear<br>
search every time some property value is needed).  Thus comment token<br>
and AST nodes were changed to contain a command ID -- index into a<br>
tables of builtin and registered commands.  Unknown commands are<br>
registered during parsing and thus are also uniformly assigned an ID.<br>
Using an ID instead of a StringRef is also a nice memory optimization<br>
since ID is a small integer that fits into a common bitfield in<br>
Comment class.<br>
<br>
This change implies that to get any information about a command (even<br>
a command name) we need a CommandTraits object to resolve the command<br>
ID to CommandInfo*.  Currently a fresh temporary CommandTraits object<br>
is created whenever it is needed since it does not have any state.<br>
But with this change it has state -- new commands can be registered,<br>
so I added a CommandTraits object to ASTContext.<br>
<br>
Also, in libclang CXComment has to be expanded to include a pointer to<br>
ASTContext so that all functions working on comment AST nodes can get<br>
a CommandTraits object.  This breaks binary compatibility of CXComment<br>
APIs.<br></blockquote><div><br>If you need to break binary compatibility, I would encourage you to go all the way and make CXComment an opaque pointer. This way further changes will never again require breaking binary compatibility (for this).<br>
<br>-- Matthieu<br> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
We can avoid breaking binary compatibility by storing CommandInfo* in<br>
AST nodes instead of unsigned command IDs (that need to be resolved).<br>
But with this approach we won't get that nice memory optimization.<br>
<br>
The patch is long, but changes are mostly boring and just change the<br>
code to use the new APIs to access command properties.  It is the<br>
architecture and binary compatibility I want to get comments about.<br>
<br>
Please review.<br>
<span class="HOEnZb"><font color="#888888"><br>
Dmitri<br>
<br>
--<br>
main(i,j){for(i=2;;i++){for(j=2;j<i;j++){if(!(i%j)){j=0;break;}}if<br>
(j){printf("%d\n",i);}}} /*Dmitri Gribenko <<a href="mailto:gribozavr@gmail.com">gribozavr@gmail.com</a>>*/<br>
</font></span><br>_______________________________________________<br>
cfe-commits mailing list<br>
<a href="mailto:cfe-commits@cs.uiuc.edu">cfe-commits@cs.uiuc.edu</a><br>
<a href="http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits</a><br>
<br></blockquote></div><br>