[PATCH] D124643: [Object][DX] Initial DXContainer parsing support

[Fangrui Song via Phabricator via llvm-commits]

MaskRay accepted this revision.
MaskRay added inline comments.
This revision is now accepted and ready to land.


================
Comment at: llvm/include/llvm/BinaryFormat/DXContainer.h:89
+} // namespace dxbc
+
+} // namespace llvm
----------------
delete blank line


================
Comment at: llvm/include/llvm/Object/DXContainer.h:22
+#include "llvm/Support/MemoryBufferRef.h"
+#include <vector>
+
----------------
Remove as the file doesn't use std::vector


================
Comment at: llvm/lib/Object/DXContainer.cpp:17
+namespace {
+Error parseFailed(const Twine &Msg) {
+  return make_error<GenericBinaryError>(Msg.str(), object_error::parse_failed);
----------------
Use static instead of anonymous namespace.

See https://llvm.org/docs/CodingStandards.html#anonymous-namespaces


================
Comment at: llvm/lib/Object/DXContainer.cpp:43
+  DXContainer Container(Object);
+  if (llvm::Error Err = Container.parseHeader())
+    return Err;
----------------
llvm::Error => Error everyewhere


================
Comment at: llvm/unittests/Object/DXContainerTest.cpp:19
+
+template <std::size_t X> MemoryBufferRef getMemoryBuffer(uint8_t Data[X]) {
+  StringRef Obj(reinterpret_cast<char *>(&Data[0]), X);
----------------
static


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D124643/new/

https://reviews.llvm.org/D124643