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.

Author: llvm-beanz

include/dxc/DxilContainer/DxilRuntimeReflection.h

@@ -237,39 +237,19 @@ class RawBytesReader {
 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 @@ class DxbcConverter : public IDxbcConverter {
     };
     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());
   }