microsoft / STL

MSVC's implementation of the C++ Standard Library.
Other
10.07k stars 1.48k forks source link

Fix warning C26818 about `switch` statements needing `default` cases #4715

Closed StephanTLavavej closed 3 months ago

StephanTLavavej commented 3 months ago

Fixes DevCom-10670613 VSO-2081258 / AB#2081258.

In general, we haven't attempted to be clean with respect to Core Guidelines warnings, as many of them are noisy and/or inappropriate for Standard Library code. However, warning C26818 "Switch statement does not cover all cases. Consider adding a 'default' label (es.79)." aligns with our conventions. We always like to have a default as a reminder to think about what happens when none of the cases are selected (this is one of the relatively few scenarios where we like to write code that isn't necessary). The only exception to this convention is when we're switching on an enum and we've handled all of the enumerators (it can still be okay to have a "can't happen" default, but sometimes we don't bother), which fortunately this warning doesn't complain about.

I added test coverage for all of these fixes, i.e. removing any fix will cause GH_002094_cpp_core_guidelines to fail.

All of the empty default: break; cases being added are used, i.e. making them abort() will cause other tests to fail.

In <format>, we have a couple of "can't happen" cases, where I've added _STL_UNREACHABLE with comments explaining why.

In <xlocmon>, we have default cases that can only be activated by users giving us a bogus money_base::pattern, so I've added _STL_ASSERT(false, "message") citing the same Standardese each time.

While auditing all of our switch statements (which is how I found half of these), I found a few cases in old code where we weren't obeying our syntax convention: a switch's last case or default must always break. Falling off of the end is an unacceptable maintenance hazard.

Finally, I removed an unnecessary, inconsistent comment in <xlocmon>.

StephanTLavavej commented 3 months ago

Sure, thanks @AlexGuteniev! :heart_eyes_cat: Pushed a commit.

StephanTLavavej commented 3 months ago

I'm speculatively mirroring this to the MSVC-internal repo - please notify me if any further changes are pushed.