[PATCH] D135764: [clang][Interp] Implement for loops

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman accepted this revision.
aaron.ballman added a comment.
This revision is now accepted and ready to land.

LGTM aside from some minor points.



================
Comment at: clang/lib/AST/Interp/ByteCodeStmtGen.cpp:361
+  this->emitLabel(IncLabel);
+  if (Inc && !this->discard(Inc))
+    return false;
----------------
What does the call to `discard()` do? Evaluate the expression and discard the results? (If so, the API might want for a better name at some point, as it can also read like "do nothing with this expression".)


================
Comment at: clang/test/AST/Interp/loops.cpp:216-226
+  constexpr int f4() {
+    int i = 0;
+    for (;;) {
+      i = i + 1;
+
+      if (i == 5)
+        break;
----------------
I'd love to see a stress test for the compile time performance of:
```
int func() {
  int i = 0;
  for (;;) {
    if (i == __INT_MAX__)
      break;
    ++i;
  }
  return i;
}
static_assert(func() == __INT_MAX__);
```
I don't think there's value to testing specifically how long it takes for this to execute, but we should probably start thinking about how to validate performance characteristics pretty soon. Perhaps relative performance compared to the existing constant expression evaluator would be interesting though?


================
Comment at: clang/test/AST/Interp/loops.cpp:271
+  ///   be rejected.
+  constexpr int f6() {
+    while(true);
----------------
Another test to add here: `for (;;);` is just as infinite.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135764