adding more tests and addressing comments

Author: joaosaffran

llvm/include/llvm/BinaryFormat/DXContainer.h

@@ -167,14 +167,12 @@ ArrayRef<EnumEntry<RootParameterType>> getRootParameterTypes();
 
 #define ROOT_PARAMETER(Val, Enum)                                              \
   case Val:                                                                    \
-    return dxbc::RootParameterType::Enum;
-inline llvm::Expected<dxbc::RootParameterType>
-safeParseParameterType(uint32_t V) {
+    return true;
+inline bool isValidParameterType(uint32_t V) {
   switch (V) {
 #include "DXContainerConstants.def"
   }
-  return createStringError(std::errc::invalid_argument,
-                           "Invalid value for parameter type");
+  return false;
 }
 
 #define SHADER_VISIBILITY(Val, Enum) Enum = Val,
@@ -186,14 +184,12 @@ ArrayRef<EnumEntry<ShaderVisibility>> getShaderVisibility();
 
 #define SHADER_VISIBILITY(Val, Enum)                                           \
   case Val:                                                                    \
-    return dxbc::ShaderVisibility::Enum;
-inline llvm::Expected<dxbc::ShaderVisibility>
-safeParseShaderVisibility(uint32_t V) {
+    return true;
+inline bool isValidShaderVisibility(uint32_t V) {
   switch (V) {
 #include "DXContainerConstants.def"
   }
-  return createStringError(std::errc::invalid_argument,
-                           "Invalid value for parameter type");
+  return false;
 }
 
 PartType parsePartType(StringRef S);

llvm/include/llvm/ObjectYAML/DXContainerYAML.h

@@ -92,6 +92,8 @@ struct RootParameterYamlDesc {
 };
 
 struct RootSignatureYamlDesc {
+  RootSignatureYamlDesc() = default;
+
   uint32_t Version;
   uint32_t NumStaticSamplers;
   uint32_t StaticSamplersOffset;
@@ -108,8 +110,6 @@ struct RootSignatureYamlDesc {
   create(const object::DirectX::RootSignature &Data);
 
 #include "llvm/BinaryFormat/DXContainerConstants.def"
-
-  RootSignatureYamlDesc() = default;
 };
 
 using ResourceFlags = dxbc::PSV::ResourceFlags;

llvm/lib/ObjectYAML/DXContainerYAML.cpp

@@ -16,7 +16,7 @@
 #include "llvm/BinaryFormat/DXContainer.h"
 #include "llvm/Support/Error.h"
 #include "llvm/Support/ScopedPrinter.h"
-#include <utility>
+#include <system_error>
 
 namespace llvm {
 
@@ -47,20 +47,17 @@ DXContainerYAML::RootSignatureYamlDesc::create(
     RootParameterYamlDesc NewP;
     NewP.Offset = PH.ParameterOffset;
 
-    llvm::Expected<dxbc::RootParameterType> TypeOrErr =
-        dxbc::safeParseParameterType(PH.ParameterType);
-    if (Error E = TypeOrErr.takeError()) {
-      return std::move(E);
-    }
+    if (!dxbc::isValidParameterType(PH.ParameterType))
+      return createStringError(std::errc::invalid_argument,
+                               "Invalid value for parameter type");
 
-    NewP.Type = TypeOrErr.get();
+    NewP.Type = (dxbc::RootParameterType)PH.ParameterType;
 
-    llvm::Expected<dxbc::ShaderVisibility> VisibilityOrErr =
-        dxbc::safeParseShaderVisibility(PH.ShaderVisibility);
-    if (Error E = VisibilityOrErr.takeError()) {
-      return std::move(E);
-    }
-    NewP.Visibility = VisibilityOrErr.get();
+    if (!dxbc::isValidShaderVisibility(PH.ShaderVisibility))
+      return createStringError(std::errc::invalid_argument,
+                               "Invalid value for shader visibility");
+
+    NewP.Visibility = (dxbc::ShaderVisibility)PH.ShaderVisibility;
 
     llvm::Expected<object::DirectX::RootParameterView> ParamViewOrErr =
         Data.getParameter(PH);

llvm/unittests/Object/DXContainerTest.cpp

@@ -8,14 +8,12 @@
 
 #include "llvm/Object/DXContainer.h"
 #include "../../tools/obj2yaml/dxcontainer2yaml.cpp"
-#include "llvm/ADT/SmallString.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/BinaryFormat/Magic.h"
 #include "llvm/ObjectYAML/DXContainerYAML.h"
 #include "llvm/ObjectYAML/yaml2obj.h"
 #include "llvm/Support/MemoryBufferRef.h"
 #include "llvm/Testing/Support/Error.h"
-#include "gmock/gmock.h"
 #include "gtest/gtest.h"
 
 using namespace llvm;
@@ -930,4 +928,24 @@ TEST(RootSignature, ParseRootConstant) {
     EXPECT_THAT_ERROR(dxcontainer2yaml(OS, getMemoryBuffer<133>(Buffer)),
                       FailedWithMessage("Invalid value for parameter type"));
   }
+  {
+    uint8_t Buffer[] = {
+        0x44, 0x58, 0x42, 0x43, 0x32, 0x9a, 0x53, 0xd8, 0xec, 0xbe, 0x35, 0x6f,
+        0x05, 0x39, 0xe1, 0xfe, 0x31, 0x20, 0xf0, 0xc1, 0x01, 0x00, 0x00, 0x00,
+        0x85, 0x00, 0x00, 0x00, 0x01, 0x00, 0x00, 0x00, 0x24, 0x00, 0x00, 0x00,
+        0x52, 0x54, 0x53, 0x30, 0x59, 0x00, 0x00, 0x00, 0x02, 0x00, 0x00, 0x00,
+        0x01, 0x00, 0x00, 0x00, 0x18, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+        0x2c, 0x00, 0x00, 0x00, 0x11, 0x00, 0x00, 0x00, 0x01, 0x00, 0x00, 0x00,
+        0xFF, 0x00, 0x00, 0x00, 0x24, 0x00, 0x00, 0x00, 0x0f, 0x00, 0x00, 0x00,
+        0x0e, 0x00, 0x00, 0x00, 0x10, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+        0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+        0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+        0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+        0x00};
+
+    SmallString<256> Storage;
+    raw_svector_ostream OS(Storage);
+    EXPECT_THAT_ERROR(dxcontainer2yaml(OS, getMemoryBuffer<133>(Buffer)),
+                      FailedWithMessage("Invalid value for shader visibility"));
+  }
 }

llvm/unittests/ObjectYAML/DXContainerYAMLTest.cpp

@@ -6,7 +6,6 @@
 //
 //===----------------------------------------------------------------------===//
 
-#include "llvm/ADT/SmallString.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/ADT/Twine.h"
 #include "llvm/ObjectYAML/ObjectYAML.h"