On Sat, 16 Nov 2024, Jonathan Wakely wrote:
void _M_erase(iterator __position) _GLIBCXX_NOEXCEPT { + if (__builtin_expect(empty(), 0)) + { + __glibcxx_requires_nonempty(); + return; + }Hmm, I'm having second thoughts about the "return without doing anything part now. For this simple test: #include <list> int main() { std::list<int> l; l.erase(l.begin()); } Currently it crashes (bad), but with -O1 there's a nice warning: /usr/include/c++/14/bits/new_allocator.h:172:33: warning: ‘void operator delete(void*, std::size_t)’ called on unallocated object ‘l’ [-Wfree-nonheap-object] And Asan can diagnose it too. Adding an assertion is definitely an improvement, as it avoids the crash . But returning when the assertion is disabled, so that the function is a no-op, means that the warning about freeing a null pointer goes away, because the compiler can see it's never reached. And now Asan can't diagnose it. I think on balance, making it a no-op and avoiding arbitrary UB is better. The warning is only possible in trivial cases where the compiler can see the pointer is definitely null. In more realistic code, there will be no warning, and UB, so turning it into a silent no-op does seem safer. If you want to detect the bug, enable assertions. What do others think? Better to add the assertion but leave the UB present when assertions are disabled, or add the assertion and silently remove the UB when assertions are enabled?
This all seems related to the current flamewar^Wdiscussion on the C++ reflectors. A precondition is violated, and the question is what we should do about it.
The ASAN regression looks bad. With special code protected by the relevant macros if necessary, it would be good if it still gave a diagnostic.
As a personal opinion, I am not convinced that no-op is a good default behavior. I am fine with UB if I am not in a debug or hardened mode. But *if* we are going to the trouble of testing the precondition, stopping the program (trap) or raising an exception seems less surprising. Ignoring the operation and continuing, which can have unpredictable behavior since the logic of the program has failed already at this point, looks like something that should only happen if the user explicitly asked for a no-fail mode that tries its best to continue even when asked to execute nonsense.
(things would obviously be different if the standard standardized the no-op behavior on these specific functions)
-- Marc Glisse
