paulkirth added a comment. Sorry, it took me a while to circle back to this.
In D135488#3931603 <https://reviews.llvm.org/D135488#3931603>, @dblaikie wrote: > I'm not sure we'd need/want to optimize a diagnostic experience for a > codebase that's got a lot of latent warnings. If people have lots of existing > warnings they should probably turn the warning off (in those instances, at > least) & I think saying "this function has a frame that's too big" without > any info is pretty hard to act on, so it doesn't seem totally outside the > realm of good diagnostics for this particular diagnostic to print a lot of > info to help a user act on it/fix the issue when they get it. If most of the > time you see this warning and want to fix it, you had to run your build again > with an extra flag - I'd say that was a bad diagnostic experience, we should > give them enough info the first time around. On the topic of how frequent warnings should be, it's important to keep in mind that a function with a large stack frame will generate a diagnostic in every caller who has inlined that function. So even if you were warning free in the last build a single function using too much stack could generate a lot of diagnostics. > If most of the time you don't need this info - yeah, that's something we > should figure out, how to provide the right amount of info to be actionable, > but in this case I suspect more-often-than-not you want some kind of > report/breakdown. If it's a case of not having a way to make it more > targeted/actionable and we just have two options ("too terse" and "too > verbose") fairly evenly split (it's not clear that most of the time one or > the other is the right answer) - I guess two different warning flags or some > kind of modifier flag could be suitable. I guess we have that for template > recursion things, maybe? Where you can ask if you want the full expansion, > but by default we give you a summarized one, skipping expansions we don't > think are relevant (I might be misremembering). While I agree that running the build again isn't ideal, I don't' see a good way to balance the need for clang to report concise errors with the need for more information in this case. Reading D127923 <https://reviews.llvm.org/D127923>, I think it's clear that many of clang's maintainers would like to keep diagnostics concise if possible. I should also note that we already give some minimal context to `-Wframe-larger-than` diagnostics by printing the breakdown of the stack usage between spills, program variables, and the UnSafeStack. I think the right approach in this case is to allow developers to opt into the behavior when needed, so your suggestion of gating it behind a flag seems like a good way to express that. I also think the diagnostic is useful enough on its own to warrant use outside of `-Wframe-larger-than`, despite being very useful when triaging those type of warnings. I guess that means I should take a harder look at plumbing this through the remarks infrastructure, even if I don't love the output. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D135488/new/ https://reviews.llvm.org/D135488 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
