ioeric added inline comments.

================
Comment at: include/clang/Sema/CodeCompleteConsumer.h:921
                                            bool IncludeBriefComments);
+  CodeCompletionString *
+  CreateCodeCompletionStringForMacro(Preprocessor &PP,
----------------
sammccall wrote:
> please document the new function - particularly why it's different and what's 
> the use case it supports
> (if I understand right, it's to allow storing CodeCompletionResult instances 
> for later stringification only when they're for macros - why?)
> 
> One of the args you're *not* taking here is ASTContext, but if I understand 
> right it must still be alive: `this->Macro` points into the IdentifierTable 
> which is owned by ASTContext.
> In some sense taking this arg seems like a safety feature!
Done. Added documentation.

>One of the args you're *not* taking here is ASTContext, but if I understand 
>right it must still be alive: this->Macro points into the IdentifierTable 
>which is owned by ASTContext.
ASTContext might not be available during preprocessing. It turned out that 
ASTContext only holds a reference to the `IdentifierTable` which is owned by 
the Preprocessor, so requiring a Preprocessor should be safe.


Repository:
  rC Clang

https://reviews.llvm.org/D48973



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to