[PATCH] D137415: [clang][Interp] Implement switch statements

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.


================
Comment at: clang/lib/AST/Interp/ByteCodeStmtGen.cpp:423
+  // Create labels and comparison ops for all case statements.
+  for (const SwitchCase *SC = S->getSwitchCaseList(); SC;
+       SC = SC->getNextSwitchCase()) {
----------------
How well does this handle large switch statements? Generated code sometimes winds up with thousands of cases, so I'm wondering if we need to have a jump table implementation for numerous cases and use the simple comparison implementation when there's only a small number of labels (and we can test to see what constitutes a good value for "small number of labels").

Also, one thing to be aware of (that hopefully won't matter TOO much in this case): the switch case list does not have a stable iteration order IIRC.


================
Comment at: clang/test/AST/Interp/switch.cpp:16-18
+  case 11:
+  case 13:
+  case 15:
----------------
lol, you should probably fix this so it's not so confusing to casual readers.


================
Comment at: clang/test/AST/Interp/switch.cpp:46
+constexpr int withInit() {
+  switch(int a = 2; a) {
+    case 1: return -1;
----------------
I think it would be good to add a non-trivial init and show that it fails when appropriate. e.g.,
```
struct Weirdo {
  consteval Weirdo(int);
  Weirdo(double);

  int Val = 1;
};

constexpr int whatever() {
  switch (Weirdo W(12); W.Val) {
  case 1: return 12;
  default: return 100;
  }
}

constexpr int whatever_else() {
  switch (Weirdo W(1.2); W.Val) { // Should get an error because of the init not being constexpr
  case 1: return 12;
  default: return 100;
  }
}

static_assert(whatever() == 12, "");
static_assert(whatever_else() == 12, ""); // Shouldn't compile because the function isn't constexpr
```


================
Comment at: clang/test/AST/Interp/switch.cpp:68
+static_assert(FT(4) == 4, "");
+static_assert(FT(5) == -1, "");
----------------
I'd like a test where the case value is itself a constant expression that is either valid or invalid:
```
constexpr int good() { return 1; }
constexpr int test(int val) {
  switch (val) {
  case good(): return 100;
  default: return -1;
  }
  return 0;
}
static_assert(test(1) == 100, "");

constexpr int bad(int val) { return val / 0; }
constexpr int another_test(int val) {
  switch (val) {
  case bad(val): return 100; // Should not compile
  default: return -1;
  }
  return 0;
}
static_assert(another_test(1) == 100, ""); // Should not compile
```


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

https://reviews.llvm.org/D137415