Allow Compiler.h to be included in C files and fix fallthrough warnings

[Nathan-Huckleberry]

Summary

Since clang does not support comment style fallthrough annotations
these should be switched to macros defined in Compiler.h. This
requires some fixing to Compiler.h.

Original patch: https://reviews.llvm.org/D66487

[aaron.ballman]

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

llvm/include/llvm/Support/Compiler.h

39–40

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.

44

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.

45

Then this can also check for defined(__has_cpp_attribute)

[Nathan-Huckleberry]

• Add comment and remove direct use of __has_cpp_attribute

[rsmith]

llvm/include/llvm/Support/Compiler.h

1–12

The file comment should explicitly call out that we allow this function to be included in both C and C++ code.

44–45

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?

[Nathan-Huckleberry]

• Clarify comments

[aaron.ballman]

Aside from commenting issue, LGTM.

llvm/include/llvm/Support/Compiler.h

44–45

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

[sfertile]

Can we please either commit this, or revert r369414 otherwise? Several of the sanitizer buildbots have been red since Tuesday due to r369414.

[Nathan-Huckleberry]

• Fix comment

[aaron.ballman]

LGTM, thank you for this!

llvm/include/llvm/Support/Compiler.h

 //===-- llvm/Support/Compiler.h - Compiler abstraction support --*- C++ -*-===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 //
 // This file defines several macros, based on the current compiler.  This allows
 // use of compiler-specific features in a way that remains portable.
 //
 //===----------------------------------------------------------------------===//
 
 #ifndef LLVM_SUPPORT_COMPILER_H
 #define LLVM_SUPPORT_COMPILER_H
 
 #include "llvm/Config/llvm-config.h"
 
+#ifdef __cplusplus
 #include <new>
+#endif
 #include <stddef.h>
 
 #if defined(_MSC_VER)
 #include <sal.h>
 #endif
 
 #ifndef __has_feature
 # define __has_feature(x) 0
 #endif
 
 #ifndef __has_extension
 # define __has_extension(x) 0
 #endif
 
 #ifndef __has_attribute
 # define __has_attribute(x) 0
 #endif
 
-#ifndef __has_cpp_attribute
-# define __has_cpp_attribute(x) 0
-#endif
-
 #ifndef __has_builtin
 # define __has_builtin(x) 0
 #endif
 
+// 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
+#if defined(__cplusplus) && defined(__has_cpp_attribute)
+# define LLVM_HAS_CPP_ATTRIBUTE(x) __has_cpp_attribute(x)
+#else
+# define LLVM_HAS_CPP_ATTRIBUTE(x) 0
+#endif
+#endif
+
 /// \macro LLVM_GNUC_PREREQ
 /// Extend the default __GNUC_PREREQ even if glibc's features.h isn't
 /// available.
 #ifndef LLVM_GNUC_PREREQ
 # if defined(__GNUC__) && defined(__GNUC_MINOR__) && defined(__GNUC_PATCHLEVEL__)
 #  define LLVM_GNUC_PREREQ(maj, min, patch) \
     ((__GNUC__ << 20) + (__GNUC_MINOR__ << 10) + __GNUC_PATCHLEVEL__ >= \
      ((maj) << 20) + ((min) << 10) + (patch))

 
 #if __has_attribute(used) || LLVM_GNUC_PREREQ(3, 1, 0)
 #define LLVM_ATTRIBUTE_USED __attribute__((__used__))
 #else
 #define LLVM_ATTRIBUTE_USED
 #endif
 
 /// LLVM_NODISCARD - Warn if a type or return value is discarded.
-#if __cplusplus > 201402L && __has_cpp_attribute(nodiscard)
+#if __cplusplus > 201402L && LLVM_HAS_CPP_ATTRIBUTE(nodiscard)
 #define LLVM_NODISCARD [[nodiscard]]
-#elif !__cplusplus
-// Workaround for llvm.org/PR23435, since clang 3.6 and below emit a spurious
-// error when __has_cpp_attribute is given a scoped attribute in C mode.
-#define LLVM_NODISCARD
-#elif __has_cpp_attribute(clang::warn_unused_result)
+#elif LLVM_HAS_CPP_ATTRIBUTE(clang::warn_unused_result)
 #define LLVM_NODISCARD [[clang::warn_unused_result]]
 #else
 #define LLVM_NODISCARD
 #endif
 
 // Indicate that a non-static, non-const C++ member function reinitializes
 // the entire object to a known state, independent of the previous state of
 // the object.
 //
 // The clang-tidy check bugprone-use-after-move recognizes this attribute as a
 // marker that a moved-from object has left the indeterminate state and can be
 // reused.
-#if __has_cpp_attribute(clang::reinitializes)
+#if LLVM_HAS_CPP_ATTRIBUTE(clang::reinitializes)
 #define LLVM_ATTRIBUTE_REINITIALIZES [[clang::reinitializes]]
 #else
 #define LLVM_ATTRIBUTE_REINITIALIZES
 #endif
 
 // Some compilers warn about unused functions. When a function is sometimes
 // used or not depending on build settings (e.g. a function only called from
 // within "assert"), this attribute can be used to suppress such warnings.

 #define LLVM_ATTRIBUTE_RETURNS_NOALIAS __attribute__((__malloc__))
 #elif defined(_MSC_VER)
 #define LLVM_ATTRIBUTE_RETURNS_NOALIAS __declspec(restrict)
 #else
 #define LLVM_ATTRIBUTE_RETURNS_NOALIAS
 #endif
 
 /// LLVM_FALLTHROUGH - Mark fallthrough cases in switch statements.
-#if __cplusplus > 201402L && __has_cpp_attribute(fallthrough)
+#if __cplusplus > 201402L && LLVM_HAS_CPP_ATTRIBUTE(fallthrough)
 #define LLVM_FALLTHROUGH [[fallthrough]]
-#elif __has_cpp_attribute(gnu::fallthrough)
+#elif LLVM_HAS_CPP_ATTRIBUTE(gnu::fallthrough)
 #define LLVM_FALLTHROUGH [[gnu::fallthrough]]
-#elif !__cplusplus
-// Workaround for llvm.org/PR23435, since clang 3.6 and below emit a spurious
-// error when __has_cpp_attribute is given a scoped attribute in C mode.
-#define LLVM_FALLTHROUGH
-#elif __has_cpp_attribute(clang::fallthrough)
+#elif __has_attribute(fallthrough)
+#define LLVM_FALLTHROUGH __attribute__((fallthrough))
+#elif LLVM_HAS_CPP_ATTRIBUTE(clang::fallthrough)
 #define LLVM_FALLTHROUGH [[clang::fallthrough]]
 #else
 #define LLVM_FALLTHROUGH
 #endif
 
 /// LLVM_REQUIRE_CONSTANT_INITIALIZATION - Apply this to globals to ensure that
 /// they are constant initialized.
-#if __has_cpp_attribute(clang::require_constant_initialization)
+#if LLVM_HAS_CPP_ATTRIBUTE(clang::require_constant_initialization)
 #define LLVM_REQUIRE_CONSTANT_INITIALIZATION                                   \
   [[clang::require_constant_initialization]]
 #else
 #define LLVM_REQUIRE_CONSTANT_INITIALIZATION
 #endif
 
 /// LLVM_EXTENSION - Support compilers where we have a keyword to suppress
 /// pedantic diagnostics.

 #if __has_feature(cxx_exceptions)
 #define LLVM_ENABLE_EXCEPTIONS 1
 #elif defined(__GNUC__) && defined(__EXCEPTIONS)
 #define LLVM_ENABLE_EXCEPTIONS 1
 #elif defined(_MSC_VER) && defined(_CPPUNWIND)
 #define LLVM_ENABLE_EXCEPTIONS 1
 #endif
 
+#ifdef __cplusplus
 namespace llvm {
 
 /// Allocate a buffer of memory with the given size and alignment.
 ///
 /// When the compiler supports aligned operator new, this will use it to to
 /// handle even over-aligned allocations.
 ///
 /// However, this doesn't make any attempt to leverage the fancier techniques
#ifdef __cpp_aligned_new
                     ,
                     std::align_val_t(Alignment)
 #endif
   );
 }
 
 } // End namespace llvm
 
+#endif // __cplusplus
 #endif

llvm/lib/Support/regcomp.c

 #include <limits.h>
 #include <stdlib.h>
 #include "regex_impl.h"
 
 #include "regutils.h"
 #include "regex2.h"
 
 #include "llvm/Config/config.h"
+#include "llvm/Support/Compiler.h"
 
 /* character-class table */
 static struct cclass {
   const char *name;
   const char *chars;
   const char *multis;
 } cclasses[] = {
   { "alnum",  "ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz\
case '\\':
     } else {
       /* Other chars are simply themselves when escaped with a backslash.
        */
       ordinary(p, c);
     }
     break;
   case '{':   /* okay as ordinary except if digit follows */
     REQUIRE(!MORE() || !isdigit((uch)PEEK()), REG_BADRPT);
-    /* fall through */
+    LLVM_FALLTHROUGH;
   default:
     ordinary(p, c);
     break;
   }
 
   if (!MORE())
     return;
   c = PEEK();
if (p->pend[i] != 0) {
       (void) dupl(p, p->pbegin[i]+1, p->pend[i]);
       EMIT(O_BACK, i);
     } else
       SETERROR(REG_ESUBREG);
     p->g->backrefs = 1;
     break;
   case '*':
     REQUIRE(starordinary, REG_BADRPT);
-    /* fall through */
+    LLVM_FALLTHROUGH;
   default:
     ordinary(p, (char)c);
     break;
   }
 
   if (EAT('*')) {   /* implemented as +? */
     /* this case does not require the (y|) trick, noKLUDGE */
     INSERT(OPLUS_, pos);
case OCH_:
         s = *scan;
         /* assert() interferes w debug printouts */
         if (OP(s) != O_QUEST && OP(s) != O_CH &&
               OP(s) != OOR2) {
           g->iflags |= REGEX_BAD;
           return;
         }
       } while (OP(s) != O_QUEST && OP(s) != O_CH);
-      /* fall through */
+      LLVM_FALLTHROUGH;
     default:    /* things that break a sequence */
       if (newlen > g->mlen) {    /* ends one */
         start = newstart;
         g->mlen = newlen;
       }
       newlen = 0;
       break;
     }