[PATCH] D88592: [DebugInfo] Emit DBG_VALUE_LIST from ISel

[David Blaikie via Phabricator via llvm-commits]

dblaikie added inline comments.


================
Comment at: llvm/lib/CodeGen/SelectionDAG/InstrEmitter.cpp:754
+    switch (Op.getKind()) {
+    default:
+      llvm_unreachable("unknown SDDbgOperand kind");
----------------
aprantl wrote:
> dblaikie wrote:
> > aprantl wrote:
> > > Is there a way to assert while still getting the benefit of Clang's switch-doesnt-cover-all-enum-cases warning here?
> > Roughly speaking: Not really, no.
> > 
> > If you have a fully covered switch, you'll get a warning if you have a default (-Wcovered-switch-default). Generally unless you're reading from user input, you shouldn't worry too much about having invalid enum values - at least most of LLVM doesn't try to have defensive asserts for these sort of spoiled enum values.
> > 
> > 
> Thanks for clarifying — I guess the subtext here is that we could just remove the assertion and rely on the compiler warning instead, right?
Perhaps more than that - if it's a fully covered switch over an enum, you can't put a default in it without breaking the -Werror build. (this was a long-standing LLVM style nit before I implemented the -Wcovered-switch-default warning so we didn't have to identify this issue manually, but it's still in https://llvm.org/docs/CodingStandards.html#don-t-use-default-labels-in-fully-covered-switches-over-enumerations ).

So, yeah: If the switch covers all the enum values, do not include a default.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D88592/new/

https://reviews.llvm.org/D88592