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

Aaron Ballman via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Aug 22 13:11:08 PDT 2019


aaron.ballman added a reviewer: rsmith.
aaron.ballman added a subscriber: rsmith.
aaron.ballman added a comment.

Adding @rsmith as a reviewer just to be sure we are not missing something.



================
Comment at: llvm/include/llvm/Support/Compiler.h:40-42
 #ifndef __has_cpp_attribute
 # define __has_cpp_attribute(x) 0
 #endif
----------------
I think that we want people to use `LLVM_HAS_CPP_ATTRIBUTE` instead of `__has_cpp_attribute` directly, so I think that this block should be removed.


================
Comment at: llvm/include/llvm/Support/Compiler.h:48
 
+#ifndef LLVM_HAS_CPP_ATTRIBUTE
+#ifdef __cplusplus
----------------
I would appreciate some comments explaining why this is needed so that someone doesn't come in and "fix" the code by removing the macro later without understanding why it's there.


================
Comment at: llvm/include/llvm/Support/Compiler.h:49
+#ifndef LLVM_HAS_CPP_ATTRIBUTE
+#ifdef __cplusplus
+# define LLVM_HAS_CPP_ATTRIBUTE(x) __has_cpp_attribute(x)
----------------
Then this can also check for `defined(__has_cpp_attribute)`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D66609





More information about the llvm-commits mailing list