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

40–42 ↗

(On Diff #216675)

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.

48 ↗

(On Diff #216675)

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.

49 ↗

(On Diff #216675)

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 ↗

(On Diff #216704)

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

44–45 ↗

(On Diff #216704)

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 ↗

(On Diff #216704)

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/trunk/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.
+// use of compiler-specific features in a way that remains portable. This header
+// can be included from either C or C++.
 //
 //===----------------------------------------------------------------------===//
 
 #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
 
+// Only use __has_cpp_attribute in C++ mode. GCC defines __has_cpp_attribute in
+// C mode, but the :: in __has_cpp_attribute(scoped::attribute) is invalid.
+#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/trunk/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;
     }