Hi Doug,

Thank you for reviewing!

On Tue, Feb 5, 2013 at 6:55 PM, Ben Gertzfield <[email protected]> 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.

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 <[email protected]>*/
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits

Reply via email to