[PATCH] D66324: clang-misexpect: Profile Guided Validation of Performance Annotations in LLVM

[Nick Desaulniers via Phabricator via cfe-commits]

nickdesaulniers added inline comments.


================
Comment at: clang/lib/Frontend/CompilerInvocation.cpp:3451
 
+const std::vector<std::string> &warnings =
+      Res.getDiagnosticOpts().Warnings;
----------------
The indentation here looks funny. Can you please run `git-clang-format` and commit the changes on top?


================
Comment at: clang/lib/Frontend/CompilerInvocation.cpp:3456
+    Res.FrontendOpts.LLVMArgs.push_back("-pgo-warn-misexpect");
+  }
+
----------------
LLVM style is no `{}` for single statement bodies.


================
Comment at: llvm/include/llvm/IR/DiagnosticInfo.h:1009
+public:
+  DiagnosticInfoMisExpect(const Function &Fn, const Twine &Msg,
+                          const DiagnosticLocation &Loc = DiagnosticLocation(),
----------------
Is this constructor used anywhere in this patch?


================
Comment at: llvm/lib/IR/DiagnosticInfo.cpp:378
+                                     Inst->getDebugLoc()),
+      Msg(Msg) {}
+
----------------
Why is this constructor defined out of line, when the other is in the header?  Seems like they should either both be in the header, or both be out of line, with the goal being consistency.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D66324