[PATCH] Add CommentOptions, allow specifying custom comment block commands

Douglas Gregor dgregor at apple.com
Mon Feb 18 15:53:40 PST 2013


On Feb 18, 2013, at 5:56 AM, Dmitri Gribenko <gribozavr at gmail.com> wrote:

> Hi Doug,
> 
> Thank you for reviewing!
> 
> On Tue, Feb 5, 2013 at 6:55 PM, Ben Gertzfield <bgertzfield at gmail.com> wrote:
>> ================
>> Comment at: lib/Frontend/ASTUnit.cpp:579-580
>> @@ -576,1 +578,4 @@
>> +    // constructed, so register them now.
>> +    Context.getCommentCommandTraits().RegisterCommentOptions(
>> +      LangOpt.CommentOpts);
>>   }
>> ----------------
>> Dmitri Gribenko wrote:
>>> As far as I see, updated() can be called multiple times.  We will get multiple commands with the same name registered.
>>> 
>>> I'm not sure that this is the correct place to put this, though.  Not all PCH loading is done through ASTUnit.  CompilerInstance::createPCHExternalASTSource might be the correct one, but I'm not sure.  I'll ask Doug.
>> I re-read the code and confirmed that updated() is called exactly once: only after both the target and language opts have been deserialized (whichever order that happens in).
>> 
>> I'm pretty confident this is a good place to read the deserialized LanguageOptions; there is no other location in the code that receives them, as far as I can tell.
> 
> Did you see this comment?  The issue is that after deserializing
> CommentOptions we need to register it in CommandTraits.  There are
> other places where we deserialize the AST, for example,
> CompilerInstance::createPCHExternalASTSource.


I saw the comment, and I think the registration is in the right place.

	- Doug




More information about the cfe-commits mailing list