[NFC] Enable -Werror for C sources too (#5021)

This fixes some fallthrough warnings that were appearing in C source
files and enables -Werror for both C and C++ sources.

Additionally, this fixes some GCC warnings and migrates AppVeyor builds
to building with clang-14 to reach agreement between GCC and Clang
about what is valid in C++.

appveyor.yml

@@ -40,7 +40,7 @@ before_build:
 build_script:
 - cmd: call utils\hct\hctbuild -%PLATFORM% -%CONFIGURATION% -show-cmake-log -spirvtest -disable-lit
 - sh: mkdir build && cd build
-- sh: cmake .. -GNinja -C ../cmake/caches/PredefinedParams.cmake -DSPIRV_BUILD_TESTS=ON -DCMAKE_BUILD_TYPE=Release -DCMAKE_C_COMPILER=clang -DCMAKE_CXX_COMPILER=clang++ -DCMAKE_CXX_FLAGS=-Werror
+- sh: cmake .. -GNinja -C ../cmake/caches/PredefinedParams.cmake -DSPIRV_BUILD_TESTS=ON -DCMAKE_BUILD_TYPE=Release -DCMAKE_C_COMPILER=clang-14 -DCMAKE_CXX_COMPILER=clang++-14 -DLLVM_ENABLE_WERROR=On
 - sh: ninja
 
 test_script:

azure-pipelines.yml

@@ -53,39 +53,37 @@ stages:
           configuration: Release
           CC: clang
           CXX: clang++
-          CXX_FLAGS: -Werror
+          CMAKE_OPTS: -DLLVM_ENABLE_WERROR=On
           OS: Linux
         Linux_Clang_Debug:
           image: ${{ variables.linux }}
           configuration: Debug
           CC: clang
           CXX: clang++
-          CXX_FLAGS: -Werror
+          CMAKE_OPTS: -DLLVM_ENABLE_WERROR=On
         Linux_Gcc_Release:
           image: ${{ variables.linux }}
           configuration: Release
           CC: gcc-9
           CXX: g++-9
-          CXX_FLAGS:
         Linux_Gcc_Debug:
           image: ${{ variables.linux }}
           configuration: Debug
           CC: gcc-9
           CXX: g++-9
-          CXX_FLAGS:
         MacOS_Clang_Release:
           image: ${{ variables.macOS }}
           configuration: Release
           CC: clang
           CXX: clang++
-          CXX_FLAGS: -Werror
+          CMAKE_OPTS: -DLLVM_ENABLE_WERROR=On
           OS: MacOS
         MacOS_Clang_Debug:
           image: ${{ variables.macOS }}
           configuration: Debug
           CC: clang
           CXX: clang++
-          CXX_FLAGS: -Werror
+          CMAKE_OPTS: -DLLVM_ENABLE_WERROR=On
 
     pool:
       vmImage: $(image)
@@ -110,7 +108,7 @@ stages:
       displayName: 'Installing dependencies'
       condition: eq(variables['image'], variables['macOS'])
     - bash: |
-        cmake -B build -G Ninja $BUILD_SOURCESDIRECTORY -DLLVM_LIT_ARGS=--xunit-xml-output=testresults.xunit.xml -DDXC_USE_LIT=On -C $BUILD_SOURCESDIRECTORY/cmake/caches/PredefinedParams.cmake -DSPIRV_BUILD_TESTS=ON -DCMAKE_BUILD_TYPE=$(configuration) -DCMAKE_C_COMPILER=$(CC) -DCMAKE_CXX_COMPILER=$(CXX) -DCMAKE_CXX_FLAGS=$(CXX_FLAGS)
+        cmake -B build -G Ninja $BUILD_SOURCESDIRECTORY -DLLVM_LIT_ARGS=--xunit-xml-output=testresults.xunit.xml -DDXC_USE_LIT=On -C $BUILD_SOURCESDIRECTORY/cmake/caches/PredefinedParams.cmake -DSPIRV_BUILD_TESTS=ON -DCMAKE_BUILD_TYPE=$(configuration) -DCMAKE_C_COMPILER=$(CC) -DCMAKE_CXX_COMPILER=$(CXX) $(CMAKE_OPTS)
       displayName: 'Running Cmake'
     - bash: |
         ninja -C build test-depends

gcp-pipelines/x86_64-linux-clang.yml

@@ -19,6 +19,7 @@ steps:
       - '-DDXC_USE_LIT=ON'
       - '-DENABLE_SPIRV_CODEGEN=ON'
       - '-DSPIRV_BUILD_TESTS=ON'
+      - '-DLLVM_ENABLE_WERROR=On'
       - '-C'
       - 'cmake/caches/PredefinedParams.cmake'
   - name: 'gcr.io/shaderc-build/shader-compiler-team:kokoro-dxc-builder'

include/llvm/Support/Compiler.h

@@ -230,6 +230,16 @@
 #define LLVM_FALLTHROUGH [[gnu::fallthrough]]
 #endif
 
+#if defined(_MSC_VER)
+#if __cplusplus > 201402L
+#define LLVM_C_FALLTHROUGH [[fallthrough]]
+#else
+#define LLVM_C_FALLTHROUGH 
+#endif
+#else
+#define LLVM_C_FALLTHROUGH  __attribute__((fallthrough));
+#endif
+
 
 /// LLVM_EXTENSION - Support compilers where we have a keyword to suppress
 /// pedantic diagnostics.

lib/Support/ConvertUTF.c

@@ -48,17 +48,12 @@
 
 
 #include "llvm/Support/ConvertUTF.h"
+#include "llvm/Support/Compiler.h" // HLSL Change - for LLVM_FALLTHROUGH
 #ifdef CVTUTF_DEBUG
 #include <stdio.h>
 #endif
 #include <assert.h>
 
-#if !defined(_WIN32) && !defined(__clang__)
-// Disable gcc's warning for fallthrough
-// this file uses it extensively without annotation
-#pragma GCC diagnostic push
-#pragma GCC diagnostic ignored "-Wimplicit-fallthrough"
-#endif
 
 static const int halfShift  = 10; /* used for shifting by 10 bits */
 
@@ -282,8 +277,11 @@ ConversionResult ConvertUTF16toUTF8 (
         }
         switch (bytesToWrite) { /* note: everything falls through. */
             case 4: *--target = (UTF8)((ch | byteMark) & byteMask); ch >>= 6;
+            LLVM_C_FALLTHROUGH; // HLSL Change
             case 3: *--target = (UTF8)((ch | byteMark) & byteMask); ch >>= 6;
+            LLVM_C_FALLTHROUGH; // HLSL Change
             case 2: *--target = (UTF8)((ch | byteMark) & byteMask); ch >>= 6;
+            LLVM_C_FALLTHROUGH; // HLSL Change
             case 1: *--target =  (UTF8)(ch | firstByteMark[bytesToWrite]);
         }
         target += bytesToWrite;
@@ -335,8 +333,11 @@ ConversionResult ConvertUTF32toUTF8 (
         }
         switch (bytesToWrite) { /* note: everything falls through. */
             case 4: *--target = (UTF8)((ch | byteMark) & byteMask); ch >>= 6;
+            LLVM_C_FALLTHROUGH; // HLSL Change
             case 3: *--target = (UTF8)((ch | byteMark) & byteMask); ch >>= 6;
+            LLVM_C_FALLTHROUGH; // HLSL Change
             case 2: *--target = (UTF8)((ch | byteMark) & byteMask); ch >>= 6;
+            LLVM_C_FALLTHROUGH; // HLSL Change
             case 1: *--target = (UTF8) (ch | firstByteMark[bytesToWrite]);
         }
         target += bytesToWrite;
@@ -366,7 +367,9 @@ static Boolean isLegalUTF8(const UTF8 *source, int length) {
     default: return false;
         /* Everything else falls through when "true"... */
     case 4: if ((a = (*--srcptr)) < 0x80 || a > 0xBF) return false;
+    LLVM_C_FALLTHROUGH; // HLSL Change
     case 3: if ((a = (*--srcptr)) < 0x80 || a > 0xBF) return false;
+    LLVM_C_FALLTHROUGH; // HLSL Change
     case 2: if ((a = (*--srcptr)) < 0x80 || a > 0xBF) return false;
 
         switch (*source) {
@@ -377,7 +380,7 @@ static Boolean isLegalUTF8(const UTF8 *source, int length) {
             case 0xF4: if (a > 0x8F) return false; break;
             default:   if (a < 0x80) return false;
         }
-
+    LLVM_C_FALLTHROUGH; // HLSL Change
     case 1: if (*source >= 0x80 && *source < 0xC2) return false;
     }
     if (*source > 0xF4) return false;
@@ -541,10 +544,15 @@ ConversionResult ConvertUTF8toUTF16 (
          */
         switch (extraBytesToRead) {
             case 5: ch += *source++; ch <<= 6; /* remember, illegal UTF-8 */
+            LLVM_C_FALLTHROUGH; // HLSL Change
             case 4: ch += *source++; ch <<= 6; /* remember, illegal UTF-8 */
+            LLVM_C_FALLTHROUGH; // HLSL Change
             case 3: ch += *source++; ch <<= 6;
+            LLVM_C_FALLTHROUGH; // HLSL Change
             case 2: ch += *source++; ch <<= 6;
+            LLVM_C_FALLTHROUGH; // HLSL Change
             case 1: ch += *source++; ch <<= 6;
+            LLVM_C_FALLTHROUGH; // HLSL Change
             case 0: ch += *source++;
         }
         ch -= offsetsFromUTF8[extraBytesToRead];
@@ -645,10 +653,15 @@ static ConversionResult ConvertUTF8toUTF32Impl(
          */
         switch (extraBytesToRead) {
             case 5: ch += *source++; ch <<= 6;
+            LLVM_C_FALLTHROUGH; // HLSL Change
             case 4: ch += *source++; ch <<= 6;
+            LLVM_C_FALLTHROUGH; // HLSL Change
             case 3: ch += *source++; ch <<= 6;
+            LLVM_C_FALLTHROUGH; // HLSL Change
             case 2: ch += *source++; ch <<= 6;
+            LLVM_C_FALLTHROUGH; // HLSL Change
             case 1: ch += *source++; ch <<= 6;
+            LLVM_C_FALLTHROUGH; // HLSL Change
             case 0: ch += *source++;
         }
         ch -= offsetsFromUTF8[extraBytesToRead];

lib/Support/DataStream.cpp

@@ -84,5 +84,5 @@ llvm::getDataFileStreamer(const std::string &Filename, std::string *StrError) {
         e.message() + "\n";
     return nullptr;
   }
-  return std::move(s);
+  return s; // HLSL Change - Fix redundant move warning.
 }

lib/Support/MemoryBuffer.cpp

@@ -373,7 +373,7 @@ getOpenFileImpl(int FD, const Twine &Filename, uint64_t FileSize,
         new (NamedBufferAlloc(Filename))
         MemoryBufferMMapFile(RequiresNullTerminator, FD, MapSize, Offset, EC));
     if (!EC)
-      return std::move(Result);
+      return Result; // HLSL Change - Fix redundant move warning.
   }
 
   std::unique_ptr<MemoryBuffer> Buf =
@@ -413,7 +413,7 @@ getOpenFileImpl(int FD, const Twine &Filename, uint64_t FileSize,
     BufPtr += NumRead;
   }
 
-  return std::move(Buf);
+  return Buf; // HLSL Change - Fix redundant move warning.
 }
 
 ErrorOr<std::unique_ptr<MemoryBuffer>>

lib/Support/YAMLTraits.cpp

@@ -351,7 +351,7 @@ std::unique_ptr<Input::HNode> Input::createHNodes(Node *N) {
         break;
       SQHNode->Entries.push_back(std::move(Entry));
     }
-    return std::move(SQHNode);
+    return SQHNode; // HLSL Change - Fix redundant move warning.
   } else if (MappingNode *Map = dyn_cast<MappingNode>(N)) {
     auto mapHNode = llvm::make_unique<MapHNode>(N);
     for (KeyValueNode &KVN : *Map) {
@@ -375,7 +375,7 @@ std::unique_ptr<Input::HNode> Input::createHNodes(Node *N) {
         break;
       mapHNode->Mapping[KeyStr] = std::move(ValueHNode);
     }
-    return std::move(mapHNode);
+    return mapHNode; // HLSL Change - Fix redundant move warning.
   } else if (isa<NullNode>(N)) {
     return llvm::make_unique<EmptyHNode>(N);
   } else {

lib/Support/regcomp.c

@@ -50,6 +50,7 @@
 #include "regcname.h"
 
 #include "llvm/Config/config.h"
+#include "llvm/Support/Compiler.h" // HLSL Change - for LLVM_C_FALLTHROUGH
 #if HAVE_STDINT_H
 #include <stdint.h>
 #else
@@ -411,6 +412,7 @@ p_ere_exp(struct parse *p)
 	case '{':		/* okay as ordinary except if digit follows */
 		REQUIRE(!MORE() || !isdigit((uch)PEEK()), REG_BADRPT);
 		/* FALLTHROUGH */
+		LLVM_C_FALLTHROUGH; // HLSL Change
 	default:
 		ordinary(p, c);
 		break;
@@ -607,6 +609,7 @@ p_simp_re(struct parse *p,
 	case '*':
 		REQUIRE(starordinary, REG_BADRPT);
 		/* FALLTHROUGH */
+		LLVM_C_FALLTHROUGH; // HLSL Change
 	default:
 		ordinary(p, (char)c);
 		break;
@@ -1512,6 +1515,7 @@ findmust(struct parse *p, struct re_guts *g)
 				}
 			} while (OP(s) != O_QUEST && OP(s) != O_CH);
 			/* fallthrough */
+			LLVM_C_FALLTHROUGH; // HLSL Change
 		default:		/* things that break a sequence */
 			if (newlen > g->mlen) {		/* ends one */
 				start = newstart;