Browse Source

[linux-port] Fix DXASSERT warnings (#1322)

The various DXASSERT macros produced a number of different warnings.
The most obvious was providing an empty string for a macro expecting
a message. Substituting in the _NOMSG macro fixes this.
Fixes 13 clang warnings.

A trickier problem for GCC and Clang is a variadic macro that is
provided no variadic arguments. Both Clang and GCC will avoid
errors by using the ##__VA_ARGS__ non-standard behavior, but it's
not an ideal solution. It turns out there aren't that many DXASSERT
macros that actually take variadic arguments. So I added an _ARGS
variant and replaced those instances with that. An ancillary benefit
is that non variadic asserts can present a message with the assert.
Fixes 583 clang warnings and 582 gcc warnings.

In what I expect was an earlier attempt to overload the DXASSERT
macro, it was defined twice each time with a different number of args.
Sadly, macros can't be overloaded, so this just produced more errors.
Fixes 69 clang warnings and 69 gcc warnings.

Finally, I removed the user declaration of std::error_code because
it is defined in a standard header <system_error>. Because clang was
including that header some places, it saw it as a redefinition. But
the Linux version is a struct and windows is a class. So nothing we
put here can make everyone happy. It doesn't really fit with this
change except for in proximity but doesn't fit anywhere else either.
Fixes 52 clang warnings
Greg Roth 7 years ago
parent
commit
0987e48b56

+ 27 - 6
include/dxc/Support/Global.h

@@ -23,6 +23,7 @@ typedef _Return_type_success_(return >= 0) long HRESULT;
 #endif // !_HRESULT_DEFINED
 
 #include <stdarg.h>
+#include <system_error>
 #include "dxc/Support/exception.h"
 
 ///////////////////////////////////////////////////////////////////////////////
@@ -72,7 +73,6 @@ struct DxcThreadMalloc {
 
 ///////////////////////////////////////////////////////////////////////////////
 // Error handling support.
-namespace std { class error_code; }
 void CheckLLVMErrorCode(const std::error_code &ec);
 
 
@@ -173,6 +173,8 @@ inline void OutputDebugFormatA(_In_ _Printf_format_string_ _Null_terminated_ con
 
 #ifdef DBG
 
+#ifdef _WIN32
+
 // DXASSERT is used to debug break when 'exp' evaluates to false and is only
 //     intended for internal developer use. It is compiled out in free
 //     builds.  This means that code that should be in the final exe should
@@ -185,25 +187,44 @@ inline void OutputDebugFormatA(_In_ _Printf_format_string_ _Null_terminated_ con
 // This prints 'Hello World (i > 10)' and breaks in the debugger if the
 // assertion doesn't hold.
 //
-#define DXASSERT(exp, fmt, ...)\
+#define DXASSERT_ARGS(exp, fmt, ...)\
   do { _Analysis_assume_(exp); if(!(exp)) {                              \
     OutputDebugFormatA("Error: \t%s\nFile:\n%s(%d)\nFunc:\t%s.\n\t" fmt "\n", "!(" #exp ")", __FILE__, __LINE__, __FUNCTION__, __VA_ARGS__); \
     __debugbreak();\
   } } while(0)
+#define DXASSERT(exp, msg) DXASSERT_ARGS(exp, msg)
 
-#define DXASSERT_LOCALVAR(local, exp, s, ...) DXASSERT(exp, s, __VA_ARGS__)
+#define DXASSERT_LOCALVAR(local, exp, msg) DXASSERT(exp, msg)
 
 #define DXASSERT_NOMSG(exp) DXASSERT(exp, "")
 
 #define DXVERIFY_NOMSG(exp) DXASSERT(exp, "")
 
-#else
+#else // _WIN32
+#include <cassert>
+
+#define DXASSERT_NOMSG assert
+
+#define DXASSERT_LOCALVAR(local, exp, msg) DXASSERT(exp, msg)
+
+#define DXVERIFY_NOMSG assert
+
+#define DXASSERT_ARGS(expr, fmt, ...) do { if (!(expr)) { fprintf(stderr, fmt, __VA_ARGS__); assert(false); } } while (0);
+
+#define DXASSERT(expr, msg) do { if (!(expr)) { fprintf(stderr, msg); assert(false && msg); } } while (0);
+
+#endif // _WIN32
+
+#else // DBG
+
+// DXASSERT_ARGS is disabled in free builds.
+#define DXASSERT_ARGS(exp, s, ...) _Analysis_assume_(exp)
 
 // DXASSERT is disabled in free builds.
-#define DXASSERT(exp, s, ...) _Analysis_assume_(exp)
+#define DXASSERT(exp, msg) _Analysis_assume_(exp)
 
 // DXASSERT_LOCALVAR is disabled in free builds, but we keep the local referenced to avoid a warning.
-#define DXASSERT_LOCALVAR(local, exp, s, ...) do { (local); _Analysis_assume_(exp); } while (0)
+#define DXASSERT_LOCALVAR(local, exp, s, ...) do { (void)(local); _Analysis_assume_(exp); } while (0)
 
 // DXASSERT_NOMSG is disabled in free builds.
 #define DXASSERT_NOMSG(exp) _Analysis_assume_(exp)

+ 1 - 1
lib/HLSL/DxilGenerationPass.cpp

@@ -1127,7 +1127,7 @@ Type *UpdateFieldTypeForLegacyLayout(Type *Ty, bool IsCBuf, DxilFieldAnnotation
       rows = matrix.Rows;
       cols = matrix.Cols;
     } else {
-      DXASSERT(matrix.Orientation == MatrixOrientation::ColumnMajor, "");
+      DXASSERT_NOMSG(matrix.Orientation == MatrixOrientation::ColumnMajor);
       cols = matrix.Rows;
       rows = matrix.Cols;
     }

+ 1 - 1
lib/HLSL/DxilSignatureElement.cpp

@@ -227,7 +227,7 @@ void DxilSignatureElement::SetCompType(CompType CT) {
 
 uint8_t DxilSignatureElement::GetColsAsMask() const {
   unsigned StartCol = IsAllocated() ? m_StartCol : 0;
-  DXASSERT(StartCol + m_Cols <= 4, "else start %u and cols %u exceed limit", StartCol, m_Cols);
+  DXASSERT_ARGS(StartCol + m_Cols <= 4, "else start %u and cols %u exceed limit", StartCol, m_Cols);
   DXASSERT(m_Cols >= 1, "else signature takes no space");
   switch (StartCol) {
   case 0: {

+ 2 - 2
lib/HLSL/HLMatrixLowerPass.cpp

@@ -958,7 +958,7 @@ void HLMatrixLowerPass::TranslateMatVecMul(CallInst *matInst,
 
   unsigned col, row;
   Type *EltTy = GetMatrixInfo(matInst->getType(), col, row);
-  DXASSERT(RVal->getType()->getVectorNumElements() == col, "");
+  DXASSERT_NOMSG(RVal->getType()->getVectorNumElements() == col);
 
   bool isFloat = EltTy->isFloatingPointTy();
 
@@ -1010,7 +1010,7 @@ void HLMatrixLowerPass::TranslateVecMatMul(CallInst *matInst,
 
   unsigned col, row;
   Type *EltTy = GetMatrixInfo(matInst->getType(), col, row);
-  DXASSERT(LVal->getType()->getVectorNumElements() == row, "");
+  DXASSERT_NOMSG(LVal->getType()->getVectorNumElements() == row);
 
   bool isFloat = EltTy->isFloatingPointTy();
 

+ 1 - 1
lib/HLSL/HLModule.cpp

@@ -839,7 +839,7 @@ void HLModule::GetParameterRowsAndCols(Type *Ty, unsigned &rows, unsigned &cols,
       rows = matrix.Rows;
       cols = matrix.Cols;
     } else {
-      DXASSERT(matrix.Orientation == MatrixOrientation::ColumnMajor, "");
+      DXASSERT_NOMSG(matrix.Orientation == MatrixOrientation::ColumnMajor);
       cols = matrix.Rows;
       rows = matrix.Cols;
     }

+ 5 - 5
lib/HLSL/HLOperationLower.cpp

@@ -1668,7 +1668,7 @@ Value *TranslateFUIBinary(CallInst *CI, IntrinsicOp IOP, OP::OpCode opcode,
       break;
     case IntrinsicOp::IOP_min:
     default:
-      DXASSERT(IOP == IntrinsicOp::IOP_min, "");
+      DXASSERT_NOMSG(IOP == IntrinsicOp::IOP_min);
       opcode = OP::OpCode::FMin;
       break;
     }
@@ -1683,7 +1683,7 @@ Value *TranslateFUITrinary(CallInst *CI, IntrinsicOp IOP, OP::OpCode opcode,
     switch (IOP) {
     case IntrinsicOp::IOP_mad:
     default:
-      DXASSERT(IOP == IntrinsicOp::IOP_mad, "");
+      DXASSERT_NOMSG(IOP == IntrinsicOp::IOP_mad);
       opcode = OP::OpCode::FMad;
       break;
     }
@@ -3944,7 +3944,7 @@ Value *TranslateProcessIsolineTessFactors(CallInst *CI, IntrinsicOp IOP, OP::OpC
                               HLOperationLowerHelper &helper,  HLObjectOperationLowerHelper *pObjHelper, bool &Translated) {
   hlsl::OP *hlslOP = &helper.hlslOP;
   // Get partition mode 
-  DXASSERT(helper.functionProps, "");
+  DXASSERT_NOMSG(helper.functionProps);
   DXASSERT(helper.functionProps->shaderKind == ShaderModel::Kind::Hull, "must be hull shader");
   DXIL::TessellatorPartitioning partition = helper.functionProps->ShaderProps.HS.partition;
   
@@ -4124,7 +4124,7 @@ Value *TranslateProcessTessFactors(CallInst *CI, IntrinsicOp IOP, OP::OpCode opc
                               HLOperationLowerHelper &helper,  HLObjectOperationLowerHelper *pObjHelper, bool &Translated) {
   hlsl::OP *hlslOP = &helper.hlslOP;
   // Get partition mode 
-  DXASSERT(helper.functionProps, "");
+  DXASSERT_NOMSG(helper.functionProps);
   DXASSERT(helper.functionProps->shaderKind == ShaderModel::Kind::Hull, "must be hull shader");
   DXIL::TessellatorPartitioning partition = helper.functionProps->ShaderProps.HS.partition;
   
@@ -6262,7 +6262,7 @@ void TranslateDefaultSubscript(CallInst *CI, HLOperationLowerHelper &helper,  HL
     } else if (GetElementPtrInst *GEP = dyn_cast<GetElementPtrInst>(user)) {
       // Must be vector type here.
       unsigned vectorSize = Ty->getVectorNumElements();
-      DXASSERT(GEP->getNumIndices() == 2, "");
+      DXASSERT_NOMSG(GEP->getNumIndices() == 2);
       Use *GEPIdx = GEP->idx_begin();
       GEPIdx++;
       Value *EltIdx = *GEPIdx;

+ 3 - 3
lib/HLSL/HLSignatureLower.cpp

@@ -889,7 +889,7 @@ void GenerateInputOutputUserCall(InputOutputAccessInfo &info, Value *undefVertex
     if (group == HLOpcodeGroup::HLIntrinsic)
       return;
     unsigned opcode = GetHLOpcode(CI);
-    DXASSERT(group == HLOpcodeGroup::HLMatLoadStore, "");
+    DXASSERT_NOMSG(group == HLOpcodeGroup::HLMatLoadStore);
     HLMatLoadStoreOpcode matOp = static_cast<HLMatLoadStoreOpcode>(opcode);
     switch (matOp) {
     case HLMatLoadStoreOpcode::ColMatLoad: {
@@ -1476,7 +1476,7 @@ void HLSignatureLower::GenerateStreamOutputOperation(Value *streamVal, unsigned
     CallInst *CI = cast<CallInst>(user);
     HLOpcodeGroup group = GetHLOpcodeGroupByName(CI->getCalledFunction());
     unsigned opcode = GetHLOpcode(CI);
-    DXASSERT_LOCALVAR(group, group == HLOpcodeGroup::HLIntrinsic, "");
+    DXASSERT_LOCALVAR(group, group == HLOpcodeGroup::HLIntrinsic, "Must be HLIntrinsic here");
     IntrinsicOp IOP = static_cast<IntrinsicOp>(opcode);
     switch (IOP) {
     case IntrinsicOp::MOP_Append:
@@ -1544,4 +1544,4 @@ void HLSignatureLower::Run() {
     GenerateDxilPatchConstantFunctionInputs();
   if (props.IsGS())
     GenerateStreamOutputOperations();
-}
+}

+ 1 - 1
lib/Transforms/Scalar/ScalarReplAggregatesHLSL.cpp

@@ -4530,7 +4530,7 @@ static unsigned AllocateSemanticIndex(
       if (matrix.Orientation == MatrixOrientation::RowMajor) {
         rows = matrix.Rows;
       } else {
-        DXASSERT(matrix.Orientation == MatrixOrientation::ColumnMajor, "");
+        DXASSERT_NOMSG(matrix.Orientation == MatrixOrientation::ColumnMajor);
         rows = matrix.Cols;
       }
     }

+ 1 - 1
tools/clang/lib/CodeGen/CGHLSLMS.cpp

@@ -5033,7 +5033,7 @@ Value *CGMSHLSLRuntime::EmitHLSLInitListExpr(CodeGenFunction &CGF, InitListExpr
   llvm::Type *RetTy = CGF.ConvertType(ResultTy);
   if (DestPtr) {
     SmallVector<Value *, 4> ParamList;
-    DXASSERT(RetTy->isAggregateType(), "");
+    DXASSERT_NOMSG(RetTy->isAggregateType());
     ParamList.emplace_back(DestPtr);
     ParamList.append(EltValList.begin(), EltValList.end());
     idx = 0;

+ 11 - 11
tools/clang/unittests/HLSL/ExecutionTest.cpp

@@ -2461,7 +2461,7 @@ public:
         return &m_table[i];
       }
     }
-    DXASSERT(false, "Invalid Table Parameter Name %s", name);
+    DXASSERT_ARGS(false, "Invalid Table Parameter Name %s", name);
     return nullptr;
   }
 
@@ -2487,7 +2487,7 @@ public:
         return &(m_table[i].m_int32Table);
       }
     }
-    DXASSERT(false, "Invalid Table Parameter Name %s", name);
+    DXASSERT_ARGS(false, "Invalid Table Parameter Name %s", name);
     return nullptr;
   }
 
@@ -2498,7 +2498,7 @@ public:
         return &(m_table[i].m_int8Table);
       }
     }
-    DXASSERT(false, "Invalid Table Parameter Name %s", name);
+    DXASSERT_ARGS(false, "Invalid Table Parameter Name %s", name);
     return nullptr;
   }
 
@@ -2509,7 +2509,7 @@ public:
         return &(m_table[i].m_int16Table);
       }
     }
-    DXASSERT(false, "Invalid Table Parameter Name %s", name);
+    DXASSERT_ARGS(false, "Invalid Table Parameter Name %s", name);
     return nullptr;
   }
 
@@ -2520,7 +2520,7 @@ public:
         return &(m_table[i].m_uint32Table);
       }
     }
-    DXASSERT(false, "Invalid Table Parameter Name %s", name);
+    DXASSERT_ARGS(false, "Invalid Table Parameter Name %s", name);
     return nullptr;
   }
 
@@ -2531,7 +2531,7 @@ public:
         return &(m_table[i].m_floatTable);
       }
     }
-    DXASSERT(false, "Invalid Table Parameter Name %s", name);
+    DXASSERT_ARGS(false, "Invalid Table Parameter Name %s", name);
     return nullptr;
   }
 
@@ -2543,7 +2543,7 @@ public:
         return &(m_table[i].m_halfTable);
       }
     }
-    DXASSERT(false, "Invalid Table Parameter Name %s", name);
+    DXASSERT_ARGS(false, "Invalid Table Parameter Name %s", name);
     return nullptr;
   }
 
@@ -2554,7 +2554,7 @@ public:
         return &(m_table[i].m_doubleTable);
       }
     }
-    DXASSERT(false, "Invalid Table Parameter Name %s", name);
+    DXASSERT_ARGS(false, "Invalid Table Parameter Name %s", name);
     return nullptr;
   }
 
@@ -2565,7 +2565,7 @@ public:
         return &(m_table[i].m_boolTable);
       }
     }
-    DXASSERT(false, "Invalid Table Parameter Name %s", name);
+    DXASSERT_ARGS(false, "Invalid Table Parameter Name %s", name);
     return nullptr;
   }
 
@@ -5030,7 +5030,7 @@ ShaderOpKind GetShaderOpKind(LPCWSTR str) {
       return ShaderOpKindTable[i].kind;
     }
   }
-  DXASSERT(false, "Invalid ShaderOp name: %s", str);
+  DXASSERT_ARGS(false, "Invalid ShaderOp name: %s", str);
   return ShaderOpKind::ShaderOpInvalid;
 }
 
@@ -5259,7 +5259,7 @@ static OutType computeExpectedWithShaderOp(const std::vector<InType> &inputs,
   case ShaderOpKind::WaveActiveAllEqual:
     return computeExpected<InType, OutType, ShaderOpKind::WaveActiveAllEqual>()(inputs, masks, maskValue, index);
   default:
-    DXASSERT(false, "Invalid ShaderOp Name: %s", str);
+    DXASSERT_ARGS(false, "Invalid ShaderOp Name: %s", str);
     return (OutType) 0;
   }
 };

+ 1 - 1
tools/clang/unittests/HLSL/ShaderOpTest.cpp

@@ -2005,7 +2005,7 @@ void ParseDataFromText(LPCWSTR pText, LPCWSTR pEnd, DXIL::ComponentType compType
     V.insert(V.end(), pB, pB + sizeof(int));
   }
   else {
-    DXASSERT(false, "Unsupported stream component type : %u", compType);
+    DXASSERT_ARGS(false, "Unsupported stream component type : %u", compType);
   }
 }