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

[Sam McCall via Phabricator via cfe-commits]

sammccall added a subscriber: rsmith.
sammccall added a comment.

In D86936#2338664 , @erichkeane wrote:

> Why did we select the 'bracket-depth' for this?  The documentation on that 
> doesn't seem to be anything close to what this should be based on:

Based on a discussion with @rsmith - the formal definition of pack expansion is 
that it expands to nested parenthesized expressions expressions.

>   Sets the limit for nested parentheses, brackets, and braces to N in blocks, 
> declarators, expressions, and struct or union declarations.
>
> The 256 default limit is REALLY small for a fold expression, particularly 
> since the instantiation depth limit is 1024 by default.  I think this patch 
> needs to be changed to use the InstantiationDepth instead.  @rjmccall, 
> thoughts?

One motivation here is to limit exposure of tools to stack overflows. A modest 
increase in the stack size of DynTypedNode triggered stack overflows in various 
traversals on real code.
While it's possible in principle to write traversals that use only data 
recursion (and I've also fixed the ones in clang itself that I can find), in 
practice a lot of code and tools rely on recursive traversals, so trivially 
creating arbitrarily deep ASTs without any limit is certain to break things 
with unclear responsibility. (Whereas this change places responsibility with 
boost numerics).

FWIW, somewhere between 1024 and 2048 was the critical depth on the systems we 
looked at, so 1024 seems... plausible, but large.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86936

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Erich Keane via Phabricator via cfe-commits]

erichkeane added a comment.

In D86936#2338664 , @erichkeane wrote:

> Why did we select the 'bracket-depth' for this?  The documentation on that 
> doesn't seem to be anything close to what this should be based on:
>
>   Sets the limit for nested parentheses, brackets, and braces to N in blocks, 
> declarators, expressions, and struct or union declarations.
>
> The 256 default limit is REALLY small for a fold expression, particularly 
> since the instantiation depth limit is 1024 by default.  I think this patch 
> needs to be changed to use the InstantiationDepth instead.  @rjmccall, 
> thoughts?

Even that limit (1024) seems like it is perhaps a little conservative. 
Apparently, this comes up in Boost Numerics, where they use a pack of size 1089 
at one point that they use a '+' fold expression that this causes us to no 
longer be able to build.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86936

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Erich Keane via Phabricator via cfe-commits]

erichkeane added subscribers: rjmccall, erichkeane.
erichkeane added a comment.

Why did we select the 'bracket-depth' for this?  The documentation on that 
doesn't seem to be anything close to what this should be based on:

  Sets the limit for nested parentheses, brackets, and braces to N in blocks, 
declarators, expressions, and struct or union declarations.

The 256 default limit is REALLY small for a fold expression, particularly since 
the instantiation depth limit is 1024 by default.  I think this patch needs to 
be changed to use the InstantiationDepth instead.  @rjmccall, thoughts?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86936

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Haojian Wu via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG9c9974c3ccb6: [clang] Limit the maximum level of fold-expr 
expansion. (authored by hokein).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86936

Files:
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/Sema/TreeTransform.h
  clang/test/SemaCXX/fold_expr_expansion_limit.cpp


Index: clang/test/SemaCXX/fold_expr_expansion_limit.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/fold_expr_expansion_limit.cpp
@@ -0,0 +1,9 @@
+// RUN: %clang_cc1 -fsyntax-only -fbracket-depth 2 -verify -std=c++17 %s
+
+template  struct seq {
+  constexpr bool zero() { return (true && ... && (V == 0)); }; // 
expected-error {{instantiating fold expression with 3 arguments exceeded 
expression nesting limit of 2}} \
+  
expected-note {{use -fbracket-depth}}
+};
+constexpr unsigned N = 3;
+auto x = __make_integer_seq{};
+static_assert(!x.zero(), ""); // expected-note {{in instantiation of member 
function}}
Index: clang/lib/Sema/TreeTransform.h
===
--- clang/lib/Sema/TreeTransform.h
+++ clang/lib/Sema/TreeTransform.h
@@ -28,6 +28,7 @@
 #include "clang/AST/StmtCXX.h"
 #include "clang/AST/StmtObjC.h"
 #include "clang/AST/StmtOpenMP.h"
+#include "clang/Basic/DiagnosticParse.h"
 #include "clang/Basic/OpenMPKinds.h"
 #include "clang/Sema/Designator.h"
 #include "clang/Sema/Lookup.h"
@@ -13193,6 +13194,18 @@
 E->getEllipsisLoc(), RHS.get(), E->getEndLoc(), NumExpansions);
   }
 
+  // Formally a fold expression expands to nested parenthesized expressions.
+  // Enforce this limit to avoid creating trees so deep we can't safely 
traverse
+  // them.
+  if (NumExpansions && SemaRef.getLangOpts().BracketDepth < NumExpansions) {
+SemaRef.Diag(E->getEllipsisLoc(),
+ clang::diag::err_fold_expression_limit_exceeded)
+<< *NumExpansions << SemaRef.getLangOpts().BracketDepth
+<< E->getSourceRange();
+SemaRef.Diag(E->getEllipsisLoc(), diag::note_bracket_depth);
+return ExprError();
+  }
+
   // The transform has determined that we should perform an elementwise
   // expansion of the pattern. Do so.
   ExprResult Result = getDerived().TransformExpr(E->getInit());
Index: clang/include/clang/Basic/DiagnosticSemaKinds.td
===
--- clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -5092,6 +5092,9 @@
   "with no fallback value">;
 def err_fold_expression_bad_operand : Error<
   "expression not permitted as operand of fold expression">;
+def err_fold_expression_limit_exceeded: Error<
+  "instantiating fold expression with %0 arguments exceeded expression nesting 
"
+  "limit of %1">, DefaultFatal, NoSFINAE;
 
 def err_unexpected_typedef : Error<
   "unexpected type name %0: expected expression">;


Index: clang/test/SemaCXX/fold_expr_expansion_limit.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/fold_expr_expansion_limit.cpp
@@ -0,0 +1,9 @@
+// RUN: %clang_cc1 -fsyntax-only -fbracket-depth 2 -verify -std=c++17 %s
+
+template  struct seq {
+  constexpr bool zero() { return (true && ... && (V == 0)); }; // expected-error {{instantiating fold expression with 3 arguments exceeded expression nesting limit of 2}} \
+  expected-note {{use -fbracket-depth}}
+};
+constexpr unsigned N = 3;
+auto x = __make_integer_seq{};
+static_assert(!x.zero(), ""); // expected-note {{in instantiation of member function}}
Index: clang/lib/Sema/TreeTransform.h
===
--- clang/lib/Sema/TreeTransform.h
+++ clang/lib/Sema/TreeTransform.h
@@ -28,6 +28,7 @@
 #include "clang/AST/StmtCXX.h"
 #include "clang/AST/StmtObjC.h"
 #include "clang/AST/StmtOpenMP.h"
+#include "clang/Basic/DiagnosticParse.h"
 #include "clang/Basic/OpenMPKinds.h"
 #include "clang/Sema/Designator.h"
 #include "clang/Sema/Lookup.h"
@@ -13193,6 +13194,18 @@
 E->getEllipsisLoc(), RHS.get(), E->getEndLoc(), NumExpansions);
   }
 
+  // Formally a fold expression expands to nested parenthesized expressions.
+  // Enforce this limit to avoid creating trees so deep we can't safely traverse
+  // them.
+  if (NumExpansions && SemaRef.getLangOpts().BracketDepth < NumExpansions) {
+SemaRef.Diag(E->getEllipsisLoc(),
+ clang::diag::err_fold_expression_limit_exceeded)
+<< *NumExpansions << SemaRef.getLangOpts().BracketDepth
+<< E->getSourceRange();
+SemaRef.Diag(E->getEllipsisLoc(), 

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

[Haojian Wu via Phabricator via cfe-commits]

hokein updated this revision to Diff 290445.
hokein added a comment.

fix a typo.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86936

Files:
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/Sema/TreeTransform.h
  clang/test/SemaCXX/fold_expr_expansion_limit.cpp


Index: clang/test/SemaCXX/fold_expr_expansion_limit.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/fold_expr_expansion_limit.cpp
@@ -0,0 +1,9 @@
+// RUN: %clang_cc1 -fsyntax-only -fbracket-depth 2 -verify -std=c++17 %s
+
+template  struct seq {
+  constexpr bool zero() { return (true && ... && (V == 0)); }; // 
expected-error {{instantiating fold expression with 3 arguments exceeded 
expression nesting limit of 2}} \
+  
expected-note {{use -fbracket-depth}}
+};
+constexpr unsigned N = 3;
+auto x = __make_integer_seq{};
+static_assert(!x.zero(), ""); // expected-note {{in instantiation of member 
function}}
Index: clang/lib/Sema/TreeTransform.h
===
--- clang/lib/Sema/TreeTransform.h
+++ clang/lib/Sema/TreeTransform.h
@@ -28,6 +28,7 @@
 #include "clang/AST/StmtCXX.h"
 #include "clang/AST/StmtObjC.h"
 #include "clang/AST/StmtOpenMP.h"
+#include "clang/Basic/DiagnosticParse.h"
 #include "clang/Basic/OpenMPKinds.h"
 #include "clang/Sema/Designator.h"
 #include "clang/Sema/Lookup.h"
@@ -13193,6 +13194,18 @@
 E->getEllipsisLoc(), RHS.get(), E->getEndLoc(), NumExpansions);
   }
 
+  // Formally a fold expression expands to nested parenthesized expressions.
+  // Enforce this limit to avoid creating trees so deep we can't safely 
traverse
+  // them.
+  if (NumExpansions && SemaRef.getLangOpts().BracketDepth < NumExpansions) {
+SemaRef.Diag(E->getEllipsisLoc(),
+ clang::diag::err_fold_expression_limit_exceeded)
+<< *NumExpansions << SemaRef.getLangOpts().BracketDepth
+<< E->getSourceRange();
+SemaRef.Diag(E->getEllipsisLoc(), diag::note_bracket_depth);
+return ExprError();
+  }
+
   // The transform has determined that we should perform an elementwise
   // expansion of the pattern. Do so.
   ExprResult Result = getDerived().TransformExpr(E->getInit());
Index: clang/include/clang/Basic/DiagnosticSemaKinds.td
===
--- clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -5092,6 +5092,9 @@
   "with no fallback value">;
 def err_fold_expression_bad_operand : Error<
   "expression not permitted as operand of fold expression">;
+def err_fold_expression_limit_exceeded: Error<
+  "instantiating fold expression with %0 arguments exceeded expression nesting 
"
+  "limit of %1">, DefaultFatal, NoSFINAE;
 
 def err_unexpected_typedef : Error<
   "unexpected type name %0: expected expression">;


Index: clang/test/SemaCXX/fold_expr_expansion_limit.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/fold_expr_expansion_limit.cpp
@@ -0,0 +1,9 @@
+// RUN: %clang_cc1 -fsyntax-only -fbracket-depth 2 -verify -std=c++17 %s
+
+template  struct seq {
+  constexpr bool zero() { return (true && ... && (V == 0)); }; // expected-error {{instantiating fold expression with 3 arguments exceeded expression nesting limit of 2}} \
+  expected-note {{use -fbracket-depth}}
+};
+constexpr unsigned N = 3;
+auto x = __make_integer_seq{};
+static_assert(!x.zero(), ""); // expected-note {{in instantiation of member function}}
Index: clang/lib/Sema/TreeTransform.h
===
--- clang/lib/Sema/TreeTransform.h
+++ clang/lib/Sema/TreeTransform.h
@@ -28,6 +28,7 @@
 #include "clang/AST/StmtCXX.h"
 #include "clang/AST/StmtObjC.h"
 #include "clang/AST/StmtOpenMP.h"
+#include "clang/Basic/DiagnosticParse.h"
 #include "clang/Basic/OpenMPKinds.h"
 #include "clang/Sema/Designator.h"
 #include "clang/Sema/Lookup.h"
@@ -13193,6 +13194,18 @@
 E->getEllipsisLoc(), RHS.get(), E->getEndLoc(), NumExpansions);
   }
 
+  // Formally a fold expression expands to nested parenthesized expressions.
+  // Enforce this limit to avoid creating trees so deep we can't safely traverse
+  // them.
+  if (NumExpansions && SemaRef.getLangOpts().BracketDepth < NumExpansions) {
+SemaRef.Diag(E->getEllipsisLoc(),
+ clang::diag::err_fold_expression_limit_exceeded)
+<< *NumExpansions << SemaRef.getLangOpts().BracketDepth
+<< E->getSourceRange();
+SemaRef.Diag(E->getEllipsisLoc(), diag::note_bracket_depth);
+return ExprError();
+  }
+
   // The transform has determined that we should perform an elementwise
   // expansion of the pattern. Do so.
   

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

[Sam McCall via Phabricator via cfe-commits]

sammccall accepted this revision.
sammccall added inline comments.
This revision is now accepted and ready to land.



Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:5096
+def err_fold_expression_limit_exceeded: Error<
+  "instantiating fold expression with %0 arguements exceeded expression 
nesting "
+  "limit of %1">, DefaultFatal, NoSFINAE;

arguements -> arguments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86936

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Haojian Wu via Phabricator via cfe-commits]

hokein added inline comments.



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)

sammccall wrote:
> you might also want to emit note_bracket_depth (I don't think you need to 
> clone it if we get the wording right)
note_bracket_depth is from parse diagnostic, introducing it in Sema feels like 
a slight layer violation, but the compile should be fine, because all 
diagnostic headers in the support library.



Comment at: clang/test/SemaCXX/fold_expr_expansion_limit.cpp:8
+auto x = __make_integer_seq{};
+static_assert(!x.zero(), ""); // expected-error {{static_assert expression is 
not an integral constant expression}} \
+ expected-note {{in instantiation of member 
function}}

sammccall wrote:
> this diagnostic is bogus :-(
> default-fatal would fix this, I guess.
oh, yeah, if a fatal error is emitted, all subsequent diagnostics will be 
silenced.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86936

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Haojian Wu via Phabricator via cfe-commits]

hokein updated this revision to Diff 289372.
hokein marked 4 inline comments as done.
hokein added a comment.

address review comments.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86936

Files:
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/Sema/TreeTransform.h
  clang/test/SemaCXX/fold_expr_expansion_limit.cpp


Index: clang/test/SemaCXX/fold_expr_expansion_limit.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/fold_expr_expansion_limit.cpp
@@ -0,0 +1,9 @@
+// RUN: %clang_cc1 -fsyntax-only -fbracket-depth 2 -verify -std=c++17 %s
+
+template  struct seq {
+  constexpr bool zero() { return (true && ... && (V == 0)); }; // 
expected-error {{instantiating fold expression with 3 arguements exceeded 
expression nesting limit of 2}} \
+  
expected-note {{use -fbracket-depth}}
+};
+constexpr unsigned N = 3;
+auto x = __make_integer_seq{};
+static_assert(!x.zero(), ""); // expected-note {{in instantiation of member 
function}}
Index: clang/lib/Sema/TreeTransform.h
===
--- clang/lib/Sema/TreeTransform.h
+++ clang/lib/Sema/TreeTransform.h
@@ -28,6 +28,7 @@
 #include "clang/AST/StmtCXX.h"
 #include "clang/AST/StmtObjC.h"
 #include "clang/AST/StmtOpenMP.h"
+#include "clang/Basic/DiagnosticParse.h"
 #include "clang/Basic/OpenMPKinds.h"
 #include "clang/Sema/Designator.h"
 #include "clang/Sema/Lookup.h"
@@ -13193,6 +13194,18 @@
 E->getEllipsisLoc(), RHS.get(), E->getEndLoc(), NumExpansions);
   }
 
+  // Formally a fold expression expands to nested parenthesized expressions.
+  // Enforce this limit to avoid creating trees so deep we can't safely 
traverse
+  // them.
+  if (NumExpansions && SemaRef.getLangOpts().BracketDepth < NumExpansions) {
+SemaRef.Diag(E->getEllipsisLoc(),
+ clang::diag::err_fold_expression_limit_exceeded)
+<< *NumExpansions << SemaRef.getLangOpts().BracketDepth
+<< E->getSourceRange();
+SemaRef.Diag(E->getEllipsisLoc(), diag::note_bracket_depth);
+return ExprError();
+  }
+
   // The transform has determined that we should perform an elementwise
   // expansion of the pattern. Do so.
   ExprResult Result = getDerived().TransformExpr(E->getInit());
Index: clang/include/clang/Basic/DiagnosticSemaKinds.td
===
--- clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -5092,6 +5092,9 @@
   "with no fallback value">;
 def err_fold_expression_bad_operand : Error<
   "expression not permitted as operand of fold expression">;
+def err_fold_expression_limit_exceeded: Error<
+  "instantiating fold expression with %0 arguements exceeded expression 
nesting "
+  "limit of %1">, DefaultFatal, NoSFINAE;
 
 def err_unexpected_typedef : Error<
   "unexpected type name %0: expected expression">;


Index: clang/test/SemaCXX/fold_expr_expansion_limit.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/fold_expr_expansion_limit.cpp
@@ -0,0 +1,9 @@
+// RUN: %clang_cc1 -fsyntax-only -fbracket-depth 2 -verify -std=c++17 %s
+
+template  struct seq {
+  constexpr bool zero() { return (true && ... && (V == 0)); }; // expected-error {{instantiating fold expression with 3 arguements exceeded expression nesting limit of 2}} \
+  expected-note {{use -fbracket-depth}}
+};
+constexpr unsigned N = 3;
+auto x = __make_integer_seq{};
+static_assert(!x.zero(), ""); // expected-note {{in instantiation of member function}}
Index: clang/lib/Sema/TreeTransform.h
===
--- clang/lib/Sema/TreeTransform.h
+++ clang/lib/Sema/TreeTransform.h
@@ -28,6 +28,7 @@
 #include "clang/AST/StmtCXX.h"
 #include "clang/AST/StmtObjC.h"
 #include "clang/AST/StmtOpenMP.h"
+#include "clang/Basic/DiagnosticParse.h"
 #include "clang/Basic/OpenMPKinds.h"
 #include "clang/Sema/Designator.h"
 #include "clang/Sema/Lookup.h"
@@ -13193,6 +13194,18 @@
 E->getEllipsisLoc(), RHS.get(), E->getEndLoc(), NumExpansions);
   }
 
+  // Formally a fold expression expands to nested parenthesized expressions.
+  // Enforce this limit to avoid creating trees so deep we can't safely traverse
+  // them.
+  if (NumExpansions && SemaRef.getLangOpts().BracketDepth < NumExpansions) {
+SemaRef.Diag(E->getEllipsisLoc(),
+ clang::diag::err_fold_expression_limit_exceeded)
+<< *NumExpansions << SemaRef.getLangOpts().BracketDepth
+<< E->getSourceRange();
+SemaRef.Diag(E->getEllipsisLoc(), diag::note_bracket_depth);
+return ExprError();
+  }
+
   // The transform has determined that we should perform an 

[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{};
+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

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Haojian Wu via Phabricator via cfe-commits]

hokein created this revision.
hokein added a reviewer: sammccall.
Herald added a project: clang.
hokein requested review of this revision.

Introduce a new diagnostic, and respect the bracket-depth (256) by default.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D86936

Files:
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/Sema/TreeTransform.h
  clang/test/SemaCXX/fold_expr_expansion_limit.cpp


Index: clang/test/SemaCXX/fold_expr_expansion_limit.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/fold_expr_expansion_limit.cpp
@@ -0,0 +1,9 @@
+// RUN: %clang_cc1 -fsyntax-only -fbracket-depth 2 -verify -std=c++17 %s
+
+template  struct seq {
+  constexpr bool zero() { return (true && ... && (V == 0)); }; // 
expected-error {{fold expression expansion level 3 exceeded maximum of 2}}
+};
+constexpr unsigned N = 3;
+auto x = __make_integer_seq{};
+static_assert(!x.zero(), ""); // expected-error {{static_assert expression is 
not an integral constant expression}} \
+ expected-note {{in instantiation of member 
function}}
Index: clang/lib/Sema/TreeTransform.h
===
--- clang/lib/Sema/TreeTransform.h
+++ clang/lib/Sema/TreeTransform.h
@@ -13192,6 +13192,13 @@
 Callee, E->getBeginLoc(), LHS.get(), E->getOperator(),
 E->getEllipsisLoc(), RHS.get(), E->getEndLoc(), NumExpansions);
   }
+  if (NumExpansions && SemaRef.getLangOpts().BracketDepth < NumExpansions) {
+SemaRef.Diag(E->getEllipsisLoc(),
+ clang::diag::err_fold_expression_expansion_exceeded)
+<< *NumExpansions << SemaRef.getLangOpts().BracketDepth
+<< E->getSourceRange();
+return ExprError();
+  }
 
   // The transform has determined that we should perform an elementwise
   // expansion of the pattern. Do so.
Index: clang/include/clang/Basic/DiagnosticSemaKinds.td
===
--- clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -5089,6 +5089,8 @@
   "with no fallback value">;
 def err_fold_expression_bad_operand : Error<
   "expression not permitted as operand of fold expression">;
+def err_fold_expression_expansion_exceeded: Error<
+  "fold expression expansion level %0 exceeded maximum of %1">;
 
 def err_unexpected_typedef : Error<
   "unexpected type name %0: expected expression">;


Index: clang/test/SemaCXX/fold_expr_expansion_limit.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/fold_expr_expansion_limit.cpp
@@ -0,0 +1,9 @@
+// RUN: %clang_cc1 -fsyntax-only -fbracket-depth 2 -verify -std=c++17 %s
+
+template  struct seq {
+  constexpr bool zero() { return (true && ... && (V == 0)); }; // expected-error {{fold expression expansion level 3 exceeded maximum of 2}}
+};
+constexpr unsigned N = 3;
+auto x = __make_integer_seq{};
+static_assert(!x.zero(), ""); // expected-error {{static_assert expression is not an integral constant expression}} \
+ expected-note {{in instantiation of member function}}
Index: clang/lib/Sema/TreeTransform.h
===
--- clang/lib/Sema/TreeTransform.h
+++ clang/lib/Sema/TreeTransform.h
@@ -13192,6 +13192,13 @@
 Callee, E->getBeginLoc(), LHS.get(), E->getOperator(),
 E->getEllipsisLoc(), RHS.get(), E->getEndLoc(), NumExpansions);
   }
+  if (NumExpansions && SemaRef.getLangOpts().BracketDepth < NumExpansions) {
+SemaRef.Diag(E->getEllipsisLoc(),
+ clang::diag::err_fold_expression_expansion_exceeded)
+<< *NumExpansions << SemaRef.getLangOpts().BracketDepth
+<< E->getSourceRange();
+return ExprError();
+  }
 
   // The transform has determined that we should perform an elementwise
   // expansion of the pattern. Do so.
Index: clang/include/clang/Basic/DiagnosticSemaKinds.td
===
--- clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -5089,6 +5089,8 @@
   "with no fallback value">;
 def err_fold_expression_bad_operand : Error<
   "expression not permitted as operand of fold expression">;
+def err_fold_expression_expansion_exceeded: Error<
+  "fold expression expansion level %0 exceeded maximum of %1">;
 
 def err_unexpected_typedef : Error<
   "unexpected type name %0: expected expression">;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits