[PATCH] D101835: [llvm] llvm-tapi-diff
Cyndy Ishida via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Tue May 4 14:38:50 PDT 2021
cishida added inline comments.
================
Comment at: llvm/include/llvm/TextAPI/Target.h:63
+std::string getTargetTriple(Target Targ);
+
----------------
nit: s/getTargetTriple/getTripleName or getTargetTripleName
Triple is already a type so this could be conflicting if there are similar functions that have that return type
================
Comment at: llvm/tools/llvm-tapi-diff/DiffEngine.h:132
+ switch (Flag) {
+ case MachO::SymbolFlags::None:
+ return "None";
----------------
I'd prefer this emit an empty string
================
Comment at: llvm/tools/llvm-tapi-diff/llvm-tapi-diff.cpp:79
+ }
+ return 1;
+}
----------------
This path should never be possible, right? Could we instead error out if `Bin*` can't be casted & early return.
and have this be `return Engine.compareFiles()`
================
Comment at: llvm/tools/llvm-tapi-diff/llvm-tapi-diff.cpp:1
+//===-- llvm-nm-diff.cpp - symbol table comparator command-line driver ---*- C++
+//-*-===//
----------------
Please update these header descriptions
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D101835/new/
https://reviews.llvm.org/D101835
More information about the llvm-commits
mailing list