[PATCH] D86936: [clang] Limit the maximum level of fold-expr expansion.

[Sam McCall via Phabricator via cfe-commits]

sammccall added inline comments.


================
Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:5092
   "expression not permitted as operand of fold expression">;
+def err_fold_expression_expansion_exceeded: Error<
+  "fold expression expansion level %0 exceeded maximum of %1">;
----------------
I think this should be NoSFINAE, similar to err_template_recursion_depth_exceeded.

Maybe DefaultFatal too?


================
Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:5093
+def err_fold_expression_expansion_exceeded: Error<
+  "fold expression expansion level %0 exceeded maximum of %1">;
 
----------------
The diagnostic test could be clearer: what's a "fold expression expansion level"?

what about "instantiating fold expression with %0 arguments exceeds expression nesting limit of %1"?


================
Comment at: clang/lib/Sema/TreeTransform.h:13195
   }
+  if (NumExpansions && SemaRef.getLangOpts().BracketDepth < NumExpansions) {
+    SemaRef.Diag(E->getEllipsisLoc(),
----------------
The brackets here are not obvious. Also we're not actually tracking bracket *depth*! So this definitely needs a comment.

Something like:
```
// Formally a fold expression expands to nested parenthesized expressions.
// Enforce this limit to avoid creating trees so deep we can't safely traverse them
```


================
Comment at: clang/lib/Sema/TreeTransform.h:13196
+  if (NumExpansions && SemaRef.getLangOpts().BracketDepth < NumExpansions) {
+    SemaRef.Diag(E->getEllipsisLoc(),
+                 clang::diag::err_fold_expression_expansion_exceeded)
----------------
you might also want to emit note_bracket_depth (I don't think you need to clone it if we get the wording right)


================
Comment at: clang/test/SemaCXX/fold_expr_expansion_limit.cpp:8
+auto x = __make_integer_seq<seq, int, N>{};
+static_assert(!x.zero(), ""); // expected-error {{static_assert expression is not an integral constant expression}} \
+                                 expected-note {{in instantiation of member function}}
----------------
this diagnostic is bogus :-(
default-fatal would fix this, I guess.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86936