[PATCH] D67253: clang-misexpect: a standalone tool for verifying the use of __builtin_expect with PGO data

[Nick Desaulniers via Phabricator via llvm-commits]

nickdesaulniers added inline comments.


================
Comment at: clang-tools-extra/clang-misexpect/tool/ClangMisExpectMain.cpp:33
+using namespace clang::tooling;
+using namespace clang::misexpect;
+using Path = std::string;
----------------
The `using` statements here should match clang-tools-extra/clang-misexpect/ClangMisExpect.cpp.


================
Comment at: clang-tools-extra/clang-misexpect/tool/ClangMisExpectMain.cpp:62
+  ExecutorName.setInitialValue("all-TUs");
+  // ExecutorName.setInitialValue("standalone");
+
----------------
accidentally committed?


================
Comment at: clang-tools-extra/clang-misexpect/tool/ClangMisExpectMain.cpp:78-79
+
+  auto &OS = llvm::errs();
+  OS << "Starting execution ... \n";
+  auto ArgAdjuster = getStripPluginsAdjuster();
----------------
I recognize the shorthand, but it seems like it could be created earlier?


================
Comment at: clang-tools-extra/clang-misexpect/tool/ClangMisExpectMain.cpp:89
+        continue;
+      }
+      AdjustedArgs.push_back(Args[I]);
----------------
unnecessary


================
Comment at: clang-tools-extra/clang-misexpect/tool/ClangMisExpectMain.cpp:116
+
+  llvm::errs() << "Execution complete\n";
+
----------------
use `OS`


================
Comment at: clang-tools-extra/clang-misexpect/tool/ClangMisExpectMain.cpp:121
+      [](llvm::StringRef Key, llvm::StringRef Value) {
+        llvm::errs() << "----" << Key.str() << "\n" << Value.str() << "\n";
+      });
----------------
Capture/use `OS`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67253