[PATCH] D141451: [clang] report inlining decisions with -Wattribute-{warning|error}

Aaron Ballman via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Tue May 2 07:03:05 PDT 2023


aaron.ballman added a comment.

In D141451#4311199 <https://reviews.llvm.org/D141451#4311199>, @dblaikie wrote:

>> probably too much, but then I wonder "how do we make Fortify work?".
>
> (I'm still sort of on the side of "if the compile-time costs to get more verbose diagnostics is high, we could have a low-quality default-on diagnostic and it could mention/recommend a different/high quality/compile-time costly diagnostic" so for code bases like the Linux kernel that rely on this sort of thing a lot, with lots of always_inlining, etc, they can tradeoff toward the better diagnostic experience and codebases that don't use these features much they can still get the checking for when it does come up, but at some cost of quality/awkwardness in needing to rebuild with other flags)

This isn't about verbosity of the diagnostics, though, so much as it's about user experience. I'd agree that enabling more verbose diagnostic checking is reasonable to ask users to opt into, but what I'm struggling with is expecting users to opt into a mode so that the diagnostics appear in the correct places instead of detached from the source they're noting. Also, with the `error` and `warning` attributes both being used by glibc (and not just for fortify source, but as a general part of the interface), I think this is broader than just inlining decision notes.

> But if folks decided some more invasive tracking was worth implementing (alongside the debug info codepath that already exists but is perhaps too slow to be always on) I wouldn't get up on a soapbox to strenuously object - I just think it's a bit unfortunate to build a duplicate thing, but maybe necessary.

Agreed on the unfortunateness of duplicating functionality; if we can avoid that, it'd be best. But I'm not sure we have more ideas on how to accomplish it, either.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141451



More information about the cfe-commits mailing list