================
Comment at: tools/clang/lib/Lex/MacroInfo.cpp:64
@@ +63,3 @@
+  for (const MacroInfo *MI = this; MI; MI = MI->PreviousDefinition) {
+    if (!SM.getFileEntryForID(SM.getFileID(MI->Location)) ||
+        SM.isBeforeInTranslationUnit(MI->Location, L))
----------------
Richard Smith wrote:
> Under what circumstances is this test necessary?
It's intended to handle a case when a macro is defined via command line. Does 
isBeforeInTranslationUnit handle this case?

================
Comment at: tools/clang/lib/Sema/AnalysisBasedWarnings.cpp:687
@@ +686,3 @@
+       it != MI->tokens_end(); ++it) {
+    StringRef S = PP.getSpelling(*it, Buf);
+    if (StringRef(P, S.size()) != S)
----------------
Richard Smith wrote:
> Alexander Kornienko wrote:
> > If this seems to be not efficient, does anyone have an idea of how to 
> > conveniently create a Token array for a known character sequence (run 
> > Lexer? fill by hand? both ways seem to be too verbose).
> > And after that, how to compare two token sequences? Manually compare 
> > getKind() and for certain kinds getIdentifierInfo(), getLiteralData(), 
> > getAnnotationValue() and some flags?
> I would recommend that you pass in a list of token values to look for, where 
> a token value is a TokenKind and (for a tok_identifier) an IdentifierInfo*, 
> rather than passing in a const char* and comparing it against the macro's 
> spelling. That would be faster, and would also catch cases where an alternate 
> spelling was used for the '[' or ']' tokens.
This only happens when we issue this specific diagnostic, so it's unlikely to 
become a real performance problem.

What I don't like in the approach you suggested, is that in order to make it 
more generic we'd have to handle not only identifiers, but at least literals 
(and maybe more?). And it is much more verbose.

Or you still think it's still better?


http://llvm-reviews.chandlerc.com/D50
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits

Reply via email to