[PATCH] D66487: Fix -Wimplicit-fallthrough warnings in regcomp.c

Aaron Ballman via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Aug 22 07:00:56 PDT 2019


aaron.ballman added a comment.

In D66487#1640188 <https://reviews.llvm.org/D66487#1640188>, @Nathan-Huckleberry wrote:

> > In D66487#1639946 <https://reviews.llvm.org/D66487#1639946>, @Nathan-Huckleberry wrote:
> > 
> >> Maybe replace __has_cpp_attribute with something like this?
> >>
> >>   #ifndef has_cpp_attribute
> >>   #ifdef __cplusplus
> >>   # define has_cpp_attribute(x) __has_cpp_attribute(x)
> >>   #else
> >>   # define has_cpp_attribute(x) 0
> >>   #endif
> >>   #endif
> >>
> > 
> > 
> > That will run into the same issue -- you'll use `has_cpp_attribute(clang::something)` and in non-C++ mode, the `::` will cause an error. You need to hide the `::` behind the `__cplusplus` check.
>
> Shouldn't the preprocessor take care of that? `has_cpp_attribute(clang::something)` just gets replaced by 0? I tested locally and it appears to work.


I think I missed the lack of double underscores there -- I was worried about the Clang 3.6 behavior commented elsewhere in the file, but that won't happen here. You're right, this code should work. I would name it `LLVM_HAS_BRACKET_ATTRIBUTE` though to avoid confusion like what I ran into. Something like:

  #ifndef LLVM_HAS_BRACKET_ATTRIBUTE
  #if defined(__cplusplus) && defined(__has_cpp_attribute)
  #define LLVM_HAS_BRACKET_ATTRIBUTE(x) __has_cpp_attribute(x)
  #else
  #define LLVM_HAS_BRACKET_ATTRIBUTE(x) 0
  #endif
  #endif

(I don't think we should name it `LLVM_HAS_ATTRIBUTE` because we also have `__has_attribute` checks for GNU-style attributes.)


Repository:
  rL LLVM

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

https://reviews.llvm.org/D66487





More information about the llvm-commits mailing list