[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