NoQ added a comment.
In D83660#2149470 <https://reviews.llvm.org/D83660#2149470>, @OikawaKirie wrote:
> It seems to be better to write a checker to report assertion failures.
No no no. No no no no no. Assertions don't say "the program crashes if this
happens", assertions say "this doesn't happen". We should trust them, not argue
against them. They're god-sent for any static analysis and also they themselves
constitute a form of dynamic analysis. Like, even if there's `assert(false)` in
the beginning of a function, we shouldn't flag it; it's merely an indication
from the user that this function is never called. There are some cases where
the assertion indeed doesn't make sense but they're extremely rare. It becomes
even more confusing when people occasionally have code to handle a would-be
assertion failure gracefully in release builds (i.e., when assertions are
disabled).
What we should warn about, though, is //precondition// failures. And the
assertion that we've stepped on here is, in fact, a precondition: "if you call
this method on an optional, //first// make sure the optional isn't empty".
I.e., the assertion is put inside the method, but in fact it should be on the
method boundary, and that's where it makes sense to warn ("method is misused").
But as of now C/C++ doesn't have a way of formally differentiating assertions
from preconditions or postconditions; such feature is known as "contracts" and
it was recently delayed until C++23. So manually teaching the static analyzer
about preconditions of `Optional`'s methods is the right solution until we have
contracts.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D83660/new/
https://reviews.llvm.org/D83660
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
