[cfe-dev] [PATCH] Automatic detection of compatibility macros for non-portable diagnostics

[Richard Smith]

Hi Alex,

Please don't add the PP callback if the fallthrough warning isn't enabled.
Also, have you made any performance measurements for this change?

I would prefer that the CompatibilityMacroFinder were given a list of token
kinds and identifier infos to look for rather than a string. That would
probably improve the performance a bit, and would allow your diagnostic to
work in cases where the tokens don't have the obvious spelling (for
instance, "<:<:clang::fallthrough:>:>").

On Tue, Jul 24, 2012 at 10:11 AM, Alexander Kornienko <alexfh at google.com>wrote:

> This patch adds an automatic detection of a compatibility macros used in
> specific projects to hide constructions based on non-portable features
> (e.g. custom C++11 attributes). It helps to adapt diagnostic messages and
> fix-it hints to use these compatibility macros instead of the actual
> construct.
>
> This feature is implemented in AnalysisBasedWarnings.cpp, as there's
> currently only one diagnostic which gets profit from this - diagnostic of
> unannotated fall-through between switch labels. But the code of the
> CompatibilityMacroFinder class was intentionally made reasonably generic,
> and doesn't contain any specific bindings to this diagnostic. The class is
> a lightweight handler of PPCallbacks::MacroDefined calls. An instance of it
> is registered via Preprocessor::addPPCallbacks for each token
> sequence (specified in plain text) to find in macros (currently only
> one). It keeps all macros with the specified expansion token sequence and
> then can determine which one can be used instead of the actual construct in
> a specific code location.
>
> A motivating example for this feature:
> There's the -Wimplicit-fallthrough warning, which detects
> [[clang::fallthrough]]; construct as an annotation of an intended
> fall-through. In projects which should be compiled both in C++11 mode and
> in C++03 mode, this construct can not be used as is, so it should be
> wrapped in a macro, e.g.:
>
> #ifdef __clang__
> #if __has_feature(cxx_attributes) &&
> __has_warning("-Wimplicit-fallthrough")
> #define LLVM_FALLTHROUGH [[clang::fallthrough]]
> #endif
> #endif
>
> #ifndef LLVM_FALLTHROUGH
> #define LLVM_FALLTHROUGH do { } while (0)
> #endif
>
>
> Prior to this feature diagnostic messages would say:
> test.cpp:156:5: *warning: unannotated fall-through between switch labels*
>     case 223:
>     ^
> test.cpp:156:5: *note: insert '[[clang::fallthrough]];' to silence this
> warning*
>     case 223:
>     ^
>     *[[clang::fallthrough]];*
> test.cpp:156:5: *note: insert 'break;' to avoid fall-through*
>     case 223:
>     ^
>     break;
>
> Applying the first of these fix-it hints will lead to broken builds in
> C++03 mode, which is usually not desired.
>
> But with the automatic detection feature the diagnostic will suggest the
> correct compatibility macro available in the corresponding source location:
> ...
> test.cpp:156:5: *note: insert 'LLVM_FALLTHROUGH;' to silence this warning*
>     case 223:
>     ^
>     *LLVM_FALLTHROUGH; *
> ...
>
> Please, review this patch. Thank you!
>
> --
> Best regards,
> Alexander Kornienko
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-dev/attachments/20120727/7c98e7d3/attachment.html>