Sirraide wrote: > My sense is that it would be weird for `-Wall` not to include > `-Wfunction-effects`, but that it would be OK for `-Wfunction-effects` not to > be enabled by default.
Hmm, hard to say; function effects are sort of a separate feature on their own, so my first reaction just now was that there’s two ways we to approach this 1. Most people probably don’t need this feature, and the people who do can just enable the warning themselves (they’ll have to know how function effects work anyway, so additionally having to know to enable the warning seems reasonable); by that logic, the warning should always be disabled by default. 2. Option 1 notwithstanding, most codebases won’t use function effect attributes, so the warning, even if active by default, will never fire (I think; see below). In codebases that *do* use the attribute, you probably would want it enabled by default so you get the warnings (otherwise, why are you using the attributes). However, option 2 seems to have more potential for problems, so I would agree that option 1 (i.e. opt-in and disabled by default, even w/ `-Wall`) is probably the way to go. As mentioned above, you already have to know how function effects work to be able to use them, so having to know to also enable the warning doesn’t seem like that much more of a burden. > I chatted with a libc++ maintainer about this. A first thought was to simply > declare __libcpp_verbose_abort() as nonblocking. But that feels like a weird > lie. Possibly most ideally, functions like this would have an attribute to > exempt them from nonblocking analysis. I feel like this might be the way to go: have some attribute that makes the analyser just ignore calls to a function entirely. I agree that synthesising it from `[[noreturn]]` seems problematic because functions that throw are definitely *not* supposed to be `nonblocking`. Going by the name of the function seems like too much of a hack to me, as that’s not really something you do in C++. As for what we’d call this attribute, maybe `ignore_in_effect_analysis`, but that’s horribly wordy. I’m not good at naming things, so maybe you can come up w/ a better idea. ;Þ (Side note: I’ll do another full review once we have all of that figured out now that I’m not sick anymore, but I expect that the rest of the implementation is probably fine as-is.) https://github.com/llvm/llvm-project/pull/99656 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
