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

[Richard Smith - zygoloid via Phabricator via llvm-commits]

rsmith accepted this revision.
rsmith added inline comments.


================
Comment at: llvm/include/llvm/Support/Compiler.h:1-12
 //===-- 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
 //
 //===----------------------------------------------------------------------===//
----------------
The file comment should explicitly call out that we allow this function to be included in both C and C++ code.


================
Comment at: llvm/include/llvm/Support/Compiler.h:44-45
 
+// 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
----------------
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?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D66609