[PATCH] D61555: Remove BinaryOpsEnd case from ConstantFoldBinaryInstruction(...)

[Cameron McInally via Phabricator via llvm-commits]

cameron.mcinally added a comment.

In D61555#1490913 <https://reviews.llvm.org/D61555#1490913>, @craig.topper wrote:

> Doesn't using BinaryOpsEnd ensure we'll get a warning if any enum value is missing from the switch in the future? Using a plain default would suppress that.


There must be something subtle that I don't understand about this. Wouldn't a default case be even stronger -- it would catch those plus any non-BinaryOp that hits the switch?

At least in *+Asserts mode, we should never see a `BinaryOpsEnd` in this function. The assert on function entry is:

  assert(Instruction::isBinaryOp(Opcode) && "Non-binary instruction detected");

Here is `isBinaryOp(...)`:

  static inline bool isBinaryOp(unsigned Opcode) {
    return Opcode >= BinaryOpsBegin && Opcode < BinaryOpsEnd;
  }

In *-Asserts mode, all bets are off. But I would think that a default case would be preferred there...


Repository:
  rL LLVM

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

https://reviews.llvm.org/D61555