[PATCH] D38379: Classify llvm-cfi-verify.

[Mitch Phillips via Phabricator via llvm-commits]

hctim added inline comments.


================
Comment at: tools/llvm-cfi-verify/FileVerifier.h:65
+  FileVerifier() = delete;
+  FileVerifier(const FileVerifier &) = delete;
+  FileVerifier(FileVerifier &&Other) = default;
----------------
vlad.tsyrklevich wrote:
> Could we also include copy & move assignment constructors
Move assignment is implemented (=default), copy assignment is deleted as each FileVerifier::Object is actually owned by the parent FileVerifier::Binary, and there is no copy construction allowed of object::Binary.


================
Comment at: tools/llvm-cfi-verify/FileVerifier.h:53
+    uint64_t InstructionSize; // Size of this instruction.
+    std::string SectionName;  // Section this instruction is found in.
+    bool Bad; // Is this instruction bad (in which case, Instr::Instruction is
----------------
vlad.tsyrklevich wrote:
> Should this be a ptr/ref to avoid duplicating memory? Or should there just be a method that gets the SectionName by iterating through the Sections instead of storing it for every Instr?
Removed (was useful in debugging).


================
Comment at: tools/llvm-cfi-verify/FileVerifier.h:54
+    std::string SectionName;  // Section this instruction is found in.
+    bool Bad; // Is this instruction bad (in which case, Instr::Instruction is
+              // invalid).
----------------
vlad.tsyrklevich wrote:
> What is bad?
Changed to Instr::Valid.


https://reviews.llvm.org/D38379