[PATCH] D110665: [clang] Don't use the AST to display backend diagnostics

[David Blaikie via Phabricator via cfe-commits]

dblaikie added inline comments.


================
Comment at: clang/lib/CodeGen/CodeGenAction.cpp:336-337
 
+      for (auto &F : getModule()->functions()) {
+        if (const Decl *FD = Gen->GetDeclForMangledName(F.getName())) {
+          auto Loc = FD->getASTContext().getFullLoc(FD->getLocation());
----------------
Could we record these earlier, partly to avoid having to do the lookup by mangled name here? (eg: when the function's created instead)


================
Comment at: clang/lib/CodeGen/CodeGenAction.cpp:642-645
+  for (auto &Pair : ManglingFullSourceLocs) {
+    if (Pair.first == Hash)
+      return Pair.second;
+  }
----------------
Not worth doing anything faster (hash table, etc)?

If it's going to stay linear - could use `llvm::find_if` perhaps - though I realize it doesn't add a /huge/ amount of value here.

If it's going to stay with the explicit loop, the Pair reference should probably be made const.


================
Comment at: clang/test/Frontend/backend-diagnostic.c:18-20
+// REGULAR: warning: stack frame size ([[#]]) exceeds limit ([[#]]) in stackSizeWarning
+// PROMOTE: error: stack frame size ([[#]]) exceeds limit ([[#]]) in stackSizeWarning
+// IGNORE-NOT: stack frame size ([[#]]) exceeds limit ([[#]]) in stackSizeWarning
----------------
I'm surprised these and other warning updates don't show the signature of the function, I'd have expected demangle to have returned something like 'stackSizeWarning()' rather than just 'stackSizeWarning'?

(also it seems nice to retain the word "function" and quotation marks around the function name in the diagnostic messages, I think? (could end up with weird situations where simple function names could be confused with prose in the error message))


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110665