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

[Timm Bäder via Phabricator via cfe-commits]

tbaeder added inline comments.


================
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:
> 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.
I know about `-fconstexpr-steps`, but the notion of "steps" is implementation dependent, isn't it? I.e. I could just cal a "step" the execution of one opcode?


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

https://reviews.llvm.org/D148614