[cfe-commits] [PATCH] Compatibility macro detection for the -Wimplicit-fallthrough diagnostic.

[Alexander Kornienko]

================
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