[PATCH] D66609: Allow Compiler.h to be included in C files and fix fallthrough warnings

Fri Aug 23 05:29:08 PDT 2019

aaron.ballman accepted this revision.
aaron.ballman added a comment.

Aside from commenting issue, LGTM.

================
Comment at: llvm/include/llvm/Support/Compiler.h:44-45

+// If included by C code, __has_cpp_attribute is undefined and scoped attributes
+// are invalid. This ensures C code does not cause errors.
+#ifndef LLVM_HAS_CPP_ATTRIBUTE
----------------
rsmith wrote:
> I don't think this comment makes it sufficiently clear what's going on here (I didn't understand until I saw the comment you deleted below). How about:
> 
> > Only use `__has_cpp_attribute` in C++, because Clang 3.6 and before reject `__has_cpp_attribute(scoped::attribute)` in C.
> 
> or something like that?
I don't think this is just the Clang 3.6 issue though. At least part of the problem is that you cannot expand `__has_cpp_attribute(clang::fallthrough)` in C mode because of the `::` in GCC. For some reason, GCC defines `__has_cpp_attribute` in C mode. https://godbolt.org/z/6JiwGv

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D66609/new/

https://reviews.llvm.org/D66609