Cleanup Windows code to conform to stricter C++ 17 compiler (#4804)

* Cleanup windows build to conform to C++ 17

MSVC's C++ language implementation deviates from the standard in a
number of ways, this patch cleans up several issues that become errors
if you build DXC using clang-cl, which has a more strict
interpreteation of C++.

This change should not have any functional impact on the final program.

Specific changes include:
* static_assert(false...) is removed, this is always ill-formed by the
C++ standard.
* Under template instantiations implicit `this` is not always allowed.
* Character array `<` comparision is... probably not what was intended.
* Implicit conversions are stricter than MSVC enforces.
* Template base types need explicit type specialization when referred
to.
* It is illegal to initialize more than one member of a union.
* Scope nested enums cannot be forward declared.
* Header paths should be case-sensitive.
* Exception specifications of specific types are disallowed in C++ 17.
* Clang is stricter about const-correctness, especially as it relates
to constant c strings.

* Update based on review feedback.

* Converted string construction to copy-constructor calls.
* Removed constexpr method bodys to cause linker error.

* Catching other methods I missed.

* I was a bit too over-zealous deleting methods.

include/dxc/DxilContainer/DxilRuntimeReflection.h

@@ -237,39 +237,19 @@ public:
 template<typename _T>
 class RecordTraits {
 public:
-  static constexpr const char *TypeName() {
-#ifdef _WIN32
-    static_assert(false, "");
-#else
-    assert(false);
-#endif
-    return nullptr;
-  }
+  static constexpr const char *TypeName();
 
-  static constexpr RuntimeDataPartType PartType() {
-#ifdef _WIN32
-    static_assert(false, "");
-#else
-    assert(false);
-#endif
-    return RuntimeDataPartType::Invalid;
-  }
+  static constexpr RuntimeDataPartType PartType();
 
   // If the following static assert is hit, it means a structure defined with
   // RDAT_STRUCT is being used in ref type, which requires the struct to have
   // a table and be defined with RDAT_STRUCT_TABLE instead.
-  static constexpr RecordTableIndex TableIndex() {
-#ifdef _WIN32
-    static_assert(false, "");
-#else
-    assert(false);
-#endif
-    return (RecordTableIndex)-1;
-  }
+  static constexpr RecordTableIndex TableIndex();
+  
   // RecordSize() is defined in order to allow for use of forward decl type in RecordRef
-  static constexpr size_t RecordSize() { /*static_assert(false, "");*/ return sizeof(_T); }
+  static constexpr size_t RecordSize() { return sizeof(_T); }
   static constexpr size_t MaxRecordSize() { return RecordTraits<_T>::DerivedRecordSize(); }
-  static constexpr size_t DerivedRecordSize() { return sizeof(_T); }
+  static constexpr size_t DerivedRecordSize();
 };
 
 ///////////////////////////////////////

include/dxc/Test/RDATDumper.h

@@ -44,19 +44,19 @@ void DumpWithBase(const hlsl::RDAT::RDATContext &ctx, DumpContext &d, const _Rec
 template<typename _RecordType>
 class RecordRefDumper : public hlsl::RDAT::RecordRef<_RecordType> {
 public:
-  RecordRefDumper(uint32_t index) { Index = index; }
+  RecordRefDumper(uint32_t index) { this->Index = index; }
   template<typename _DumpTy = _RecordType>
   const char *TypeName(const hlsl::RDAT::RDATContext &ctx) const {
-    if (const char *name = RecordRefDumper<_DumpTy>(Index).TypeNameDerived(ctx))
+    if (const char *name = RecordRefDumper<_DumpTy>(this->Index).TypeNameDerived(ctx))
       return name;
-    RecordRef<_DumpTy> rr = { Index };
+    RecordRef<_DumpTy> rr = { this->Index };
     if (rr.Get(ctx))
       return RecordTraits<_DumpTy>::TypeName();
     return nullptr;
   }
   template<typename _DumpTy = _RecordType>
   void Dump(const hlsl::RDAT::RDATContext &ctx, DumpContext &d) const {
-    RecordRefDumper<_DumpTy> rrDumper(Index);
+    RecordRefDumper<_DumpTy> rrDumper(this->Index);
     if (const _DumpTy *ptr = rrDumper.Get(ctx)) {
       static_cast< const RecordDumper<_DumpTy>* >(ptr)->Dump(ctx, d);
       rrDumper.DumpDerived(ctx, d);

lib/DxcSupport/dxcapi.use.cpp

@@ -33,7 +33,7 @@ static std::string GetWin32ErrorMessage(DWORD err) {
   DWORD formattedMsgLen =
       FormatMessageA(FORMAT_MESSAGE_FROM_SYSTEM | FORMAT_MESSAGE_IGNORE_INSERTS,
                     nullptr, err, 0, formattedMsg, _countof(formattedMsg), 0);
-  if (formattedMsg > 0 && formattedMsgLen < _countof(formattedMsg)) {
+  if (formattedMsgLen > 0 && formattedMsgLen < _countof(formattedMsg)) {
     TrimEOL(formattedMsg);
     return std::string(formattedMsg);
   }

lib/DxilDia/DxcPixTypes.cpp

@@ -275,7 +275,7 @@ STDMETHODIMP dxil_debug_info::DxcPixStructType::GetFieldByName(
     _In_ LPCWSTR lpName,
     _Outptr_result_z_ IDxcPixStructField **ppField)
 {
-  std::string name = CW2A(lpName);
+  std::string name = std::string(CW2A(lpName));
   for (auto *Node : m_pStruct->getElements())
   {
     auto* pDIField = llvm::dyn_cast<llvm::DIDerivedType>(Node);

lib/DxilDia/DxcPixVariables.cpp

@@ -208,7 +208,7 @@ STDMETHODIMP dxil_debug_info::DxcPixDxilLiveVariables::GetVariableByName(
     _In_ LPCWSTR Name,
     _Outptr_result_z_ IDxcPixVariable **ppVariable)
 {
-  std::string name = CW2A(Name);
+  std::string name = std::string(CW2A(Name));
 
   for (auto *VarInfo : m_LiveVars)
   {

lib/DxilDia/DxilDiaSymbolManager.cpp

@@ -103,7 +103,7 @@ struct DISymbol : public Symbol {
 
 template <typename N>
 struct TypedSymbol : public DISymbol<N> {
-  TypedSymbol(IMalloc *M, N Node, DWORD dwTypeID, llvm::DIType *Type) : DISymbol(M, Node), m_dwTypeID(dwTypeID), m_pType(Type) {}
+  TypedSymbol(IMalloc *M, N Node, DWORD dwTypeID, llvm::DIType *Type) : DISymbol<N>(M, Node), m_dwTypeID(dwTypeID), m_pType(Type) {}
 
   STDMETHODIMP get_type(
     /* [retval][out] */ IDiaSymbol **ppRetVal) override {

lib/Support/Windows/Program.inc

@@ -97,7 +97,7 @@ ErrorOr<std::string> sys::findProgramByName(StringRef Name,
 
   return std::string(U8Result.begin(), U8Result.end());
 #else
-  return "";
+  return std::string("");
 #endif
 }
 

projects/dxilconv/lib/DxbcConverter/DxbcConverterImpl.h

@@ -407,12 +407,9 @@ protected:
     };
     vector<pair<unsigned, BasicBlock*> > SwitchCases;  // Switch
 
-    Scope() : Kind(Kind::Function), pPreScopeBB(nullptr), pPostScopeBB(nullptr), NameIndex(0), 
-              pThenBB(nullptr), pElseBB(nullptr), pCond(nullptr),
-              pLoopBB(nullptr), ContinueIndex(0), LoopBreakIndex(0),
-              pDefaultBB(nullptr), pSelector(nullptr), CaseGroupIndex(0), SwitchBreakIndex(0),
-              LabelIdx(0), CallIdx(0), ReturnTokenOffset(0), ReturnIndex(0), bEntryFunc(false),
-              pHullLoopBB(nullptr), HullLoopBreakIndex(0), pInductionVar(nullptr), HullLoopTripCount(0) {}
+    Scope() : Kind(Kind::Function), pPreScopeBB(nullptr), pPostScopeBB(nullptr), NameIndex(0) {
+      memset(reinterpret_cast<char*>(&pThenBB), '\0', reinterpret_cast<char*>(&SwitchCases) - reinterpret_cast<char*>(&pThenBB));
+    }
 
     void SetEntry(bool b = true) { DXASSERT_NOMSG(Kind==Function); bEntryFunc = b; }
     bool IsEntry() const { DXASSERT_NOMSG(Kind==Function); return bEntryFunc; }

projects/dxilconv/lib/DxbcConverter/DxbcUtil.h

@@ -19,12 +19,12 @@
 #include "dxc/DXIL/DxilResource.h"
 #include "dxc/DXIL/DxilConstants.h"
 
+#include "llvm/IR/Instructions.h"
+
 namespace llvm {
 class Type;
 class LLVMContext;
 class Value;
-class AtomicRMWInst;
-enum AtomicRMWInst::BinOp;
 }
 
 #define DXASSERT_DXBC(__exp) DXASSERT(__exp, "otherwise incorrect assumption about DXBC")

projects/dxilconv/unittests/DxilConvTests.cpp

@@ -188,7 +188,7 @@ TEST_F(DxilConvTest, ManualFileCheckTest) {
   WEX::Common::String value;
   VERIFY_SUCCEEDED(RuntimeParameters::TryGetValue(L"InputPath", value));
 
-  std::wstring path = value;
+  std::wstring path = static_cast<const wchar_t*>(value);
   if (!llvm::sys::path::is_absolute(CW2A(path.c_str()).m_psz)) {
     path = hlsl_test::GetPathToHlslDataFile(path.c_str());
   }

tools/clang/tools/dxa/dxa.cpp

@@ -18,7 +18,7 @@
 #include "dxc/Support/HLSLOptions.h"
 #include "dxc/DxilContainer/DxilContainer.h"
 #include "dxc/DxilRootSignature/DxilRootSignature.h"
-#include "dxc/test/RDATDumper.h"
+#include "dxc/Test/RDATDumper.h"
 
 #include "llvm/Support/CommandLine.h"
 #include "llvm/Support//MSFileSystem.h"

tools/clang/tools/dxcompiler/DXCompiler.cpp

@@ -29,7 +29,7 @@ HRESULT SetupRegistryPassForPIX();
 
 #ifdef LLVM_ON_WIN32
 // operator new and friends.
-void *  __CRTDECL operator new(std::size_t size) throw(std::bad_alloc) {
+void *  __CRTDECL operator new(std::size_t size) noexcept(false) {
   void * ptr = DxcGetThreadMallocNoRef()->Alloc(size);
   if (ptr == nullptr)
     throw std::bad_alloc();

tools/clang/tools/dxcompiler/dxclinker.cpp

@@ -300,7 +300,7 @@ HRESULT STDMETHODCALLTYPE DxcLinker::Link(
       }
     }
     DiagStream.flush();
-    CComPtr<IStream> pStream = pDiagStream;
+    CComPtr<IStream> pStream = static_cast<CComPtr<IStream>>(pDiagStream);
     dxcutil::CreateOperationResultFromOutputs(pOutputBlob, pStream, warnings,
                                               hasErrorOccurred, ppResult);
   }

tools/clang/tools/dxlib-sample/dxlib_sample.cpp

@@ -21,7 +21,7 @@ using namespace hlsl;
 #pragma warning( disable : 4290 )
 
 // operator new and friends.
-void *  __CRTDECL operator new(std::size_t size) throw(std::bad_alloc) {
+void *  __CRTDECL operator new(std::size_t size) noexcept(false) {
   void * ptr = DxcGetThreadMallocNoRef()->Alloc(size);
   if (ptr == nullptr)
     throw std::bad_alloc();

tools/clang/tools/dxrfallbackcompiler/DXCompiler.cpp

@@ -23,7 +23,7 @@ namespace hlsl { HRESULT SetupRegistryPassForHLSL(); }
 #pragma warning( disable : 4290 )
 
 // operator new and friends.
-void *  __CRTDECL operator new(std::size_t size) throw(std::bad_alloc) {
+void *  __CRTDECL operator new(std::size_t size) noexcept(false) {
   void * ptr = DxcGetThreadMallocNoRef()->Alloc(size);
   if (ptr == nullptr)
     throw std::bad_alloc();

tools/clang/tools/dxrfallbackcompiler/dxcdxrfallbackcompiler.cpp

@@ -331,7 +331,7 @@ HRESULT STDMETHODCALLTYPE DxcDxrFallbackCompiler::RenameAndLink(
         }
 
         DiagStream.flush();
-        CComPtr<IStream> pStream = pDiagStream;
+        CComPtr<IStream> pStream = static_cast<CComPtr<IStream>>(pDiagStream);
         std::string warnings;
         dxcutil::CreateOperationResultFromOutputs(pResultBlob, pStream, warnings, hasErrors, ppResult);
     }
@@ -415,7 +415,7 @@ HRESULT STDMETHODCALLTYPE DxcDxrFallbackCompiler::PatchShaderBindingTables(
         }
 
         DiagStream.flush();
-        CComPtr<IStream> pStream = pDiagStream;
+        CComPtr<IStream> pStream = static_cast<CComPtr<IStream>>(pDiagStream);
         std::string warnings;
         dxcutil::CreateOperationResultFromOutputs(pResultBlob, pStream, warnings, false, ppResult);
     }
@@ -574,7 +574,7 @@ HRESULT STDMETHODCALLTYPE DxcDxrFallbackCompiler::Link(
         }
 
         DiagStream.flush();
-        CComPtr<IStream> pStream = pDiagStream;
+        CComPtr<IStream> pStream = static_cast<CComPtr<IStream>>(pDiagStream);
         std::string warnings;
         dxcutil::CreateOperationResultFromOutputs(pResultBlob, pStream, warnings, hasErrors, ppResult);
     }
@@ -729,7 +729,7 @@ HRESULT STDMETHODCALLTYPE DxcDxrFallbackCompiler::Compile(
     }
 
     DiagStream.flush();
-    CComPtr<IStream> pStream = pDiagStream;
+    CComPtr<IStream> pStream = static_cast<CComPtr<IStream>>(pDiagStream);
     std::string warnings;
     dxcutil::CreateOperationResultFromOutputs(pResultBlob, pStream, warnings, hasErrors, ppResult);
 

tools/clang/unittests/HLSL/CompilerTest.cpp

@@ -1854,7 +1854,7 @@ TEST_F(CompilerTest, CompileThenTestPdbUtilsWarningOpt) {
 
     CComBSTR pMainFileName;
     VERIFY_SUCCEEDED(pPdbUtils->GetMainFileName(&pMainFileName));
-    std::wstring mainFileName = pMainFileName;
+    std::wstring mainFileName = static_cast<const wchar_t*>(pMainFileName);
     VERIFY_ARE_EQUAL(mainFileName, L"hlsl.hlsl");
   };
 
@@ -4037,7 +4037,7 @@ TEST_F(CompilerTest, DISABLED_ManualFileCheckTest) {
   WEX::Common::String value;
   VERIFY_SUCCEEDED(RuntimeParameters::TryGetValue(L"InputPath", value));
 
-  std::wstring path = value;
+  std::wstring path = static_cast<const wchar_t*>(value);
   if (!llvm::sys::path::is_absolute(CW2A(path.c_str()).m_psz)) {
     path = hlsl_test::GetPathToHlslDataFile(path.c_str());
   }

tools/clang/unittests/HLSL/ExecutionTest.cpp

@@ -624,7 +624,7 @@ public:
   void VerifyRawBufferLdStTestResults(const std::shared_ptr<st::ShaderOpTest> test, const RawBufferLdStTestData<Ty> &testData);
                                       
   bool SetupRawBufferLdStTest(D3D_SHADER_MODEL shaderModel, RawBufferLdStType dataType, CComPtr<ID3D12Device> &pDevice, 
-                              CComPtr<IStream> &pStream, char *&sTy, char *&additionalOptions);
+                              CComPtr<IStream> &pStream, const char *&sTy, const char *&additionalOptions);
 
   template <class Ty>
   void RunBasicShaderModelTest(CComPtr<ID3D12Device> pDevice, const char *pShaderModelStr, const char *pShader, Ty *pInputDataPairs, unsigned inputDataCount);
@@ -9225,7 +9225,7 @@ TEST_F(ExecutionTest, GraphicsRawBufferLdStHalf) {
 
 bool ExecutionTest::SetupRawBufferLdStTest(D3D_SHADER_MODEL shaderModel, RawBufferLdStType dataType,
                                            CComPtr<ID3D12Device> &pDevice, CComPtr<IStream> &pStream, 
-                                           char *&sTy, char *&additionalOptions) {
+                                           const char *&sTy, const char *&additionalOptions) {
   if (!CreateDevice(&pDevice, shaderModel)) {
     return false;
   }
@@ -9333,7 +9333,8 @@ void ExecutionTest::RunComputeRawBufferLdStTest(D3D_SHADER_MODEL shaderModel, Ra
 
    CComPtr<ID3D12Device> pDevice;
    CComPtr<IStream> pStream;
-   char *sTy = nullptr, *additionalOptions = nullptr;
+   const char *sTy = nullptr;
+   const char *additionalOptions = nullptr;
 
    if (!SetupRawBufferLdStTest(shaderModel, dataType, pDevice, pStream, sTy, additionalOptions)) {
      return;
@@ -9373,7 +9374,8 @@ void ExecutionTest::RunGraphicsRawBufferLdStTest(D3D_SHADER_MODEL shaderModel, R
 
   CComPtr<ID3D12Device> pDevice;
   CComPtr<IStream> pStream;
-  char *sTy = nullptr, *additionalOptions = nullptr;
+  const char *sTy = nullptr;
+  const char *additionalOptions = nullptr;
 
   if (!SetupRawBufferLdStTest(shaderModel, dataType, pDevice, pStream, sTy, additionalOptions)) {
     return;

tools/clang/unittests/HLSLTestLib/RDATDumper.cpp

@@ -10,7 +10,7 @@
 ///////////////////////////////////////////////////////////////////////////////
 
 #include "dxc/Support/Global.h"
-#include "dxc/test/RDATDumper.h"
+#include "dxc/Test/RDATDumper.h"
 
 using namespace hlsl;
 using namespace RDAT;

tools/dxexp/dxexp.cpp

@@ -152,21 +152,21 @@ typedef struct D3D12_FEATURE_DATA_D3D12_OPTIONS14
 
 #define DXEXP_HIGHEST_SHADER_MODEL D3D_SHADER_MODEL_6_8
 
-static char *BoolToStrJson(bool value) {
+static const char *BoolToStrJson(bool value) {
   return value ? "true" : "false";
 }
 
-static char *BoolToStrText(bool value) {
+static const char *BoolToStrText(bool value) {
   return value ? "YES" : "NO";
 }
 
-static char *(*BoolToStr)(bool value);
+static const char *(*BoolToStr)(bool value);
 static bool IsOutputJson;
 
 #define json_printf(...) if (IsOutputJson) { printf(__VA_ARGS__); }
 #define text_printf(...) if (!IsOutputJson) { printf(__VA_ARGS__); }
 
-static char *ShaderModelToStr(D3D_SHADER_MODEL SM) {
+static const char *ShaderModelToStr(D3D_SHADER_MODEL SM) {
   switch ((UINT32)SM) {
   case D3D_SHADER_MODEL_5_1: return "5.1";
   case D3D_SHADER_MODEL_6_0: return "6.0";