[PATCH] D148614: [clang][Interp] Add frame depth checking

[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, though I would appreciate adding the other test case from my comments since it's interesting behavior.



================
Comment at: clang/lib/AST/Interp/Interp.cpp:345-352
+  if ((S.Current->getDepth() + 1) > S.getLangOpts().ConstexprCallDepth) {
+    S.FFDiag(S.Current->getSource(OpPC),
+             diag::note_constexpr_depth_limit_exceeded)
+        << S.getLangOpts().ConstexprCallDepth;
+    return false;
+  }
+
----------------
aaron.ballman wrote:
> `-fconstexpr-depth` sets the number of *recursive* constexpr calls, but this looks like it's measuring the depth of the call stack regardless of whether there's recursion or not. Can you add a test where you set this value to something low and make nested calls that exceed that depth?
> 
> (Also, there's `-fconstexpr-steps` https://clang.llvm.org/docs/UsersManual.html#cmdoption-fconstexpr-steps we'll need to support at some point, in case you weren't aware of the option.)
Whelp, TIL that our docs are wrong and should be updated (I'll take care of that): https://godbolt.org/z/ahPjPnhGr

It has nothing to do with recursion, that's just the way in which you'd typically run into it.


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

https://reviews.llvm.org/D148614