aeubanks 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());
----------------
dblaikie wrote:
> Could we record these earlier, partly to avoid having to do the lookup by
> mangled name here? (eg: when the function's created instead)
We don't know if a function will be emitted into the IR when creating its AST
node. And `GetDeclForMangledName` may not work if we haven't fully constructed
the AST yet (not sure about this, but this would definitely require a lot of
frontend knowledge).
================
Comment at: clang/lib/CodeGen/CodeGenAction.cpp:642-645
+ for (auto &Pair : ManglingFullSourceLocs) {
+ if (Pair.first == Hash)
+ return Pair.second;
+ }
----------------
dblaikie wrote:
> 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.
yeah, `find_if` doesn't really make it any easier to read/shorter
I'm trying to optimize for construction time, not reporting time since
reporting warnings/errors is much rarer. Can't get much better than a vector.
================
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
----------------
dblaikie wrote:
> 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))
I've added quotes.
For the word "function", sometimes the demangling prints out actual prose, e.g.
in backend-stack-frame-diagnostics.cpp below, we have a `invocation function
for block in frameSizeBlocksWarning()`, and adding "function" would print
`function 'invocation function for block in frameSizeBlocksWarning()'`. Maybe
that's fine since those are rarer, WDYT?
For C functions, I believe since there's no overloading, we don't print
types/parentheses.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D110665/new/
https://reviews.llvm.org/D110665
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
