On Feb 18, 2013, at 5:56 AM, Dmitri Gribenko <[email protected]> wrote:
> 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. I saw the comment, and I think the registration is in the right place. - Doug _______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
