Mark expected, unexpected, and ALL exception types as [[nodiscard]] by StephanTLavavej · Pull Request #5174 · microsoft/STL (original) (raw)

A reddit thread has persuaded me to explore marking entire types as [[nodiscard]], which previously we reserved for guard types only.

We're very careful about avoiding [[nodiscard]] false positives. Marking an entire type affects all user-defined functions returning that type (as well as all temporary constructions), with no [[discard]] antidote available for function declarations. (Individual callsites can always be (void) suppressed.) For existing types like C++11 error_code, I continue to believe that too much time has passed for it to be reasonable to mark the entire type now.

☑️ <expected> ⛔

However, while explaining our original decision again, I remembered that <expected> is actually still new to C++23. I forgot because we shipped it in VS 2022 17.3 (Aug 2022), over two millennia years ago. As it's a new type, and we haven't finalized C++23, we can set policies for this new type without fear of introducing a blizzard of [[nodiscard]] warnings in legacy codebases. Early adopters will be writing their first functions returning std::expected, so we can keep all of their callsites clean from day one.

I am marking expected<T, E>, expected<void, E>, and unexpected<E>. The idea is that as expected is an alternative to exception handling, it should be difficult to unintentionally ignore errors. If a user-defined function has chosen to return the expected type, all callers should be inspecting the return value. We are essentially saying that all user-defined functions returning expected should be marked [[nodiscard]], and are making that decision for all of them, for all time. (Marking unexpected is going to be the least useful because it'll be used the least, but as it contains error information that's intended to construct an expected, the same arguments about not dropping it on the floor apply.)

❕ Exception types

A different rationale applies here. These are legacy types, many going back to C++98, but they are rarely used as values. If a function is returning an exception type by value, it is very likely a "maker" function (crafting a string literal, etc.). Returning an exception by value and then discarding it is highly suspicious, as it looks like it was meant to be thrown. Similarly, constructing a temporary exception and dropping it on the floor is extremely likely to be a bug.

The following code previously compiled without warnings (except C4702 "unreachable code", which is noisy and might be silenced). Now it emits a warning, just like a discarded guard temporary:

D:\GitHub\STL\out\x64>type meow.cpp

#include #include using namespace std;

void woof() { println("woof() one!"); runtime_error{"TOO MANY PUPPIES"}; println("woof() two!"); }

int main() { try { println("main() before!"); woof(); println("main() after!"); } catch (const runtime_error& e) { println("runtime_error: {}", e.what()); } }

D:\GitHub\STL\out\x64>cl /EHsc /nologo /W4 /wd4702 /std:c++latest /MTd /Od meow.cpp
meow.cpp
meow.cpp(7): warning C4834: discarding return value of function with [[nodiscard]] attribute

✅ Test updates

💯 The exception hierarchy

I believe I've audited our entire exception hierarchy, including internal exceptions. If I missed anything in product code, please let me know:

⚠️ Note to self when mirroring

Internally, I've also marked the following types that are defined in VCRuntime. (I've found and marked all of them, although I still need to build and test those changes, which I'll do while mirroring.)