================
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)
----------------
Alexander Kornienko wrote:
> 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?
I still think it's better to test the sequence of tokens themselves rather than
forming a string from them and testing that. As another example of things that
can go wrong with your current approach, it matches (note the space):
#define BROKEN_FALLTHROUGH_MACRO [[clang::fall through]]
I would suggest that you don't worry about matching literals until/unless you
have a use case for that.
================
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))
----------------
Alexander Kornienko wrote:
> 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?
Yes, isBeforeInTranslationUnit should handle that.
http://llvm-reviews.chandlerc.com/D50
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
