[PATCH] D28510: Reinstate CWG1607 restrictions on lambdas appearing inside certain constant-expressions
rsmith added a comment.
In https://reviews.llvm.org/D28510#654821, @faisalv wrote:
> In https://reviews.llvm.org/D28510#653794, @rsmith wrote:
>
> > I don't think it's possible to check this in the way you're doing so here.
> > In general, there's no way to know whether a constant expression will be
> > part of a `typedef` declaration or function declaration until you've
> > finished parsing it (when you're parsing the decl-specifiers in a
> > declaration you don't know whether you're declaring a function or a
> > variable, and the `typedef` keyword might appear later).
>
>
>
>
>
> I see. The issue is that the current approach would forbid valid variable
> declarations such as:
>
> void (*f)(int [([]{return 5;}())]) = 0;
>
> ... where lambdas can appear within the array declarators (I believe that's
> the only syntactic neighborhood that can cause this problem, right?).
I think so, at least until Louis removes the restriction on lambdas in
unevaluated operands. Then we have a whole host of new problems:
decltype([]{}()) typedef x; // error, lambda in a typedef
template decltype([]{}()) f(); // error, lambda in a function
declaration
template decltype([]{}()) x; // ok
If we want a forward-looking change which prepares us for that, we should be
thinking about how to deal with deferring the check until we get to the end of
the declaration and find out whether we declared a function or a typedef.
> Well lambda's can't appear in unevaluated operands yet, so your example would
> be ill-formed? If so, we don't have to worry about them showing up in
> decl-specifiers?
> The only Declarator where they could be well formed is within an array
> declarator of a variable or a parameter of a function pointer variable (but
> no where else, i.e typedefs and function declarations), right?
Right. But we should at least keep in mind the upcoming removal of the
unevaluated operands restriction. Ideally, we would get some implementation
experience with that now, so we can make sure that we standardize a reasonable
set of rules.
https://reviews.llvm.org/D28510
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D28510: Reinstate CWG1607 restrictions on lambdas appearing inside certain constant-expressions
faisalv updated this revision to Diff 85927.
faisalv added a comment.
I tried a different approach, not because I was convinced it was better, but
because it seemed (at the time) a simpler tweak - I'm not sure it is robust
enough - but would appreciate your thoughts on it.
The approach is predicated on the following:
- lambda expressions can only appear in types in a non-unevaluated context,
within array brackets
- if we track within each parameter declarator, its parent function declarator,
we can always get to the outer most declarator and if the outermost declarator
has the form (ptr-op f)(...) it can not be a function declarator (but is a
variable) - we can determine this without waiting for the recursion to complete
(I set a special token on the declarator once we start parsing a parens
declarator and the token is a ptr-operator)
- additionally we can check the context of the outermost declarator to ensure
it is not within an alias-declaration or a template-argument
Based on the test cases, it seems to work well - but not sure if the test cases
are exhaustive.
Also the patch needs some cleanup if we agree that this direction is worth
pursuing (and not too fragile an approach).
Thanks!
Would appreciate feedback!
https://reviews.llvm.org/D28510
Files:
include/clang/Basic/DiagnosticSemaKinds.td
include/clang/Parse/Parser.h
include/clang/Sema/DeclSpec.h
include/clang/Sema/Sema.h
lib/Parse/ParseDecl.cpp
lib/Parse/ParseExpr.cpp
lib/Parse/ParseTemplate.cpp
lib/Sema/SemaExpr.cpp
lib/Sema/SemaExprMember.cpp
lib/Sema/SemaLambda.cpp
lib/Sema/SemaTemplate.cpp
lib/Sema/SemaTemplateInstantiateDecl.cpp
lib/Sema/TreeTransform.h
test/SemaCXX/cxx1z-constexpr-lambdas.cpp
Index: test/SemaCXX/cxx1z-constexpr-lambdas.cpp
===
--- test/SemaCXX/cxx1z-constexpr-lambdas.cpp
+++ test/SemaCXX/cxx1z-constexpr-lambdas.cpp
@@ -157,6 +157,56 @@
} // end ns1_simple_lambda
+namespace test_forbidden_lambda_expressions {
+
+template struct X { }; //expected-error{{lambda expression may not appear}}
+X<[]{return 10; }()> x; //expected-error{{lambda expression may not appear}}
+void f(int arr[([] { return 5; }())]); //expected-error{{lambda expression may not appear}}
+// FIXME: Should this be ok?
+auto L = [](int arr[([] { return 5; }())]) { }; // OK
+
+// These should be allowed:
+struct A {
+ int : ([] { return 5; }());
+};
+
+int arr[([] { return 5; }())];
+enum { E = [] { return 5; }() };
+static_assert([]{return 5; }() == 5);
+int *ip = new int[([] { return 5; })()];
+
+int test_case(int x) {
+ switch(x) {
+case [] { return 5; }(): //OK
+ break;
+case [] (auto a) { return a; }(6): //expected-note{{previous}}
+ break;
+case 6: //expected-error{{duplicate}}
+ break;
+ }
+ return x;
+}
+namespace ns2 {
+int (*f)(int a[([] { return 5; }())]);
+int fun(int a[([] { return 5; }())]); //expected-error{{lambda expression}}
+int fun2(void (*)(int a[([] { return 5; }())])); //expected-error{{lambda expression}}
+int (*fun2p)(void f(int a[([] { return 5; }())]));
+
+int (*g(int))[([] { return 5; })()]; //expected-error{{lambda expression}}
+int *(fun3)(int a[([] { return 5; }())]); //expected-error{{lambda expression}}
+
+
+int *(&fun3r)(int a[([] { return 5; }())]) = ([] () -> auto& { int *fv(int *); return fv; })();
+
+int (*g2(int))[([] { return 5; })()]; //expected-error{{lambda expression}}
+
+int (*(*g3)(int))[([] { return 5; })()];
+template struct X { };
+X x; //expected-error{{lambda expression}}
+} //end ns2
+
+} // end ns forbidden_lambda_expressions
+
namespace ns1_unimplemented {
namespace ns1_captures {
constexpr auto f(int i) {
Index: lib/Sema/TreeTransform.h
===
--- lib/Sema/TreeTransform.h
+++ lib/Sema/TreeTransform.h
@@ -3787,7 +3787,8 @@
case TemplateArgument::Expression: {
// Template argument expressions are constant expressions.
EnterExpressionEvaluationContext Unevaluated(
-getSema(), Uneval ? Sema::Unevaluated : Sema::ConstantEvaluated);
+getSema(),
+Uneval ? Sema::Unevaluated : Sema::ConstantEvaluatedInTemplateArgument);
Expr *InputExpr = Input.getSourceExpression();
if (!InputExpr) InputExpr = Input.getArgument().getAsExpr();
Index: lib/Sema/SemaTemplateInstantiateDecl.cpp
===
--- lib/Sema/SemaTemplateInstantiateDecl.cpp
+++ lib/Sema/SemaTemplateInstantiateDecl.cpp
@@ -2236,8 +2236,8 @@
Param->setInvalidDecl();
if (D->hasDefaultArgument() && !D->defaultArgumentWasInherited()) {
-EnterExpressionEvaluationContext ConstantEvaluated(SemaRef,
- Sema::ConstantEvaluated);
+EnterExpressionEvaluationContext ConstantEvaluated(
+SemaRef, Sema::ConstantEvaluatedInTemplateArgument);
ExprResult Value = SemaRef.SubstExpr(
[PATCH] D28510: Reinstate CWG1607 restrictions on lambdas appearing inside certain constant-expressions
faisalv added a comment.
In https://reviews.llvm.org/D28510#653794, @rsmith wrote:
> I don't think it's possible to check this in the way you're doing so here. In
> general, there's no way to know whether a constant expression will be part of
> a `typedef` declaration or function declaration until you've finished parsing
> it (when you're parsing the decl-specifiers in a declaration you don't know
> whether you're declaring a function or a variable, and the `typedef` keyword
> might appear later).
I see. The issue is that the current approach would forbid valid variable
declarations such as:
void (*f)(int [([]{return 5;}())]) = 0;
... where lambdas can appear within the array declarators (I believe that's the
only syntactic neighborhood that can cause this problem, right?).
> So I think you need a different approach here. How about tracking the set of
> contained lambdas on the `Declarator` and `DeclSpec` objects, and diagnose
> from `ActOnFunctionDeclarator` / `ActOnTypedefDeclarator` if the current
> expression evaluation context contains any lambdas? (Maybe when entering an
> expression evaluation context, pass an optional `SmallVectorImpl*` to
> `Sema` to collect the lambdas contained within the expression.)
Yes - I can see something along these lines working well...
> There are some particularly "fun" cases to watch out for here:
>
> decltype([]{})
> a, // ok
> f(); // ill-formed
>
>
> ... that require us to track the lambdas in the `DeclSpec` separately from
> the lambdas in the `Declarator`.
Well lambda's can't appear in unevaluated operands yet, so your example would
be ill-formed? If so, we don't have to worry about them showing up in
decl-specifiers?
The only Declarator where they could be well formed is within an array
declarator of a variable or a parameter of a function pointer variable (but no
where else, i.e typedefs and function declarations), right?
Thanks!
Repository:
rL LLVM
https://reviews.llvm.org/D28510
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D28510: Reinstate CWG1607 restrictions on lambdas appearing inside certain constant-expressions
rsmith requested changes to this revision.
rsmith added a comment.
This revision now requires changes to proceed.
I don't think it's possible to check this in the way you're doing so here. In
general, there's no way to know whether a constant expression will be part of a
`typedef` declaration or function declaration until you've finished parsing it
(when you're parsing the decl-specifiers in a declaration you don't know
whether you're declaring a function or a variable, and the `typedef` keyword
might appear later).
So I think you need a different approach here. How about tracking the set of
contained lambdas on the `Declarator` and `DeclSpec` objects, and diagnose from
`ActOnFunctionDeclarator` / `ActOnTypedefDeclarator` if the current expression
evaluation context contains any lambdas? (Maybe when entering an expression
evaluation context, pass an optional `SmallVectorImpl*` to `Sema` to
collect the lambdas contained within the expression.)
There are some particularly "fun" cases to watch out for here:
decltype([]{})
a, // ok
f(); // ill-formed
... that require us to track the lambdas in the `DeclSpec` separately from the
lambdas in the `Declarator`.
Repository:
rL LLVM
https://reviews.llvm.org/D28510
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D28510: Reinstate CWG1607 restrictions on lambdas appearing inside certain constant-expressions
faisalv updated this revision to Diff 84476.
faisalv added a comment.
The updated patch adds two additional enumerators to
ExpressionEvaluationContext: ConstantEvaluatedInTemplateArgument and
ConstantEvaluatedInFunctionSignature and sets them appropriately (as opposed to
our previous approach of setting the IsLambdaExpressionForbidden flag in those
locations).
When popping off the EvaluationContext, instead of checking the flag, we now
check the EvaluationContext directly to determine if the Lambda Expression is
valid.
Repository:
rL LLVM
https://reviews.llvm.org/D28510
Files:
include/clang/Basic/DiagnosticSemaKinds.td
include/clang/Parse/Parser.h
include/clang/Sema/Sema.h
lib/Parse/ParseDecl.cpp
lib/Parse/ParseExpr.cpp
lib/Parse/ParseTemplate.cpp
lib/Sema/SemaExpr.cpp
lib/Sema/SemaExprMember.cpp
lib/Sema/SemaLambda.cpp
lib/Sema/SemaTemplate.cpp
lib/Sema/SemaTemplateInstantiateDecl.cpp
lib/Sema/TreeTransform.h
test/SemaCXX/cxx1z-constexpr-lambdas.cpp
Index: test/SemaCXX/cxx1z-constexpr-lambdas.cpp
===
--- test/SemaCXX/cxx1z-constexpr-lambdas.cpp
+++ test/SemaCXX/cxx1z-constexpr-lambdas.cpp
@@ -157,6 +157,38 @@
} // end ns1_simple_lambda
+namespace test_forbidden_lambda_expressions {
+
+template struct X { }; //expected-error{{lambda expression may not appear}}
+X<[]{return 10; }()> x; //expected-error{{lambda expression may not appear}}
+void f(int arr[([] { return 5; }())]); //expected-error{{lambda expression may not appear}}
+// FIXME: Should this be ok?
+auto L = [](int arr[([] { return 5; }())]) { }; // OK
+
+// These should be allowed:
+struct A {
+ int : ([] { return 5; }());
+};
+
+int arr[([] { return 5; }())];
+enum { E = [] { return 5; }() };
+static_assert([]{return 5; }() == 5);
+int *ip = new int[([] { return 5; })()];
+
+int test_case(int x) {
+ switch(x) {
+case [] { return 5; }(): //OK
+ break;
+case [] (auto a) { return a; }(6): //expected-note{{previous}}
+ break;
+case 6: //expected-error{{duplicate}}
+ break;
+ }
+ return x;
+}
+
+} // end ns forbidden_lambda_expressions
+
namespace ns1_unimplemented {
namespace ns1_captures {
constexpr auto f(int i) {
Index: lib/Sema/TreeTransform.h
===
--- lib/Sema/TreeTransform.h
+++ lib/Sema/TreeTransform.h
@@ -3781,7 +3781,8 @@
case TemplateArgument::Expression: {
// Template argument expressions are constant expressions.
EnterExpressionEvaluationContext Unevaluated(
-getSema(), Uneval ? Sema::Unevaluated : Sema::ConstantEvaluated);
+getSema(),
+Uneval ? Sema::Unevaluated : Sema::ConstantEvaluatedInTemplateArgument);
Expr *InputExpr = Input.getSourceExpression();
if (!InputExpr) InputExpr = Input.getArgument().getAsExpr();
Index: lib/Sema/SemaTemplateInstantiateDecl.cpp
===
--- lib/Sema/SemaTemplateInstantiateDecl.cpp
+++ lib/Sema/SemaTemplateInstantiateDecl.cpp
@@ -2236,8 +2236,8 @@
Param->setInvalidDecl();
if (D->hasDefaultArgument() && !D->defaultArgumentWasInherited()) {
-EnterExpressionEvaluationContext ConstantEvaluated(SemaRef,
- Sema::ConstantEvaluated);
+EnterExpressionEvaluationContext ConstantEvaluated(
+SemaRef, Sema::ConstantEvaluatedInTemplateArgument);
ExprResult Value = SemaRef.SubstExpr(D->getDefaultArgument(), TemplateArgs);
if (!Value.isInvalid())
Param->setDefaultArgument(Value.get());
Index: lib/Sema/SemaTemplate.cpp
===
--- lib/Sema/SemaTemplate.cpp
+++ lib/Sema/SemaTemplate.cpp
@@ -3520,8 +3520,8 @@
for (unsigned i = 0, e = Param->getDepth(); i != e; ++i)
TemplateArgLists.addOuterTemplateArguments(None);
- EnterExpressionEvaluationContext ConstantEvaluated(SemaRef,
- Sema::ConstantEvaluated);
+ EnterExpressionEvaluationContext ConstantEvaluated(
+ SemaRef, Sema::ConstantEvaluatedInTemplateArgument);
return SemaRef.SubstExpr(Param->getDefaultArgument(), TemplateArgLists);
}
@@ -5152,8 +5152,8 @@
// The initialization of the parameter from the argument is
// a constant-evaluated context.
- EnterExpressionEvaluationContext ConstantEvaluated(*this,
- Sema::ConstantEvaluated);
+ EnterExpressionEvaluationContext ConstantEvaluated(
+ *this, Sema::ConstantEvaluatedInTemplateArgument);
if (getLangOpts().CPlusPlus1z) {
// C++1z [temp.arg.nontype]p1:
Index: lib/Sema/SemaLambda.cpp
===
--- lib/Sema/SemaLambda.cpp
+++ lib/Sema/SemaLambda.cpp
@@ -1591,11 +1591,12 @@
// evaluation of e, following the rules of the abstract machine, would
/
[PATCH] D28510: Reinstate CWG1607 restrictions on lambdas appearing inside certain constant-expressions
faisalv added inline comments.
Comment at: include/clang/Sema/Sema.h:894-895
+/// \brief Whether lambda expressions are forbidden here.
+bool IsLambdaExprForbidden;
+
rsmith wrote:
> Rather than adding a flag, how about we have two different kinds of
> `ExpressionEvaluationContext` for constant expressions: a generic "constant
> expression" context and a "constant expression in signature" context?
But not all lambdas that appear in template arguments are in signatures - so
should we split it into ConstantEvaluated,
ConstantEvaluatedInSignatureOrTemplateArg ?
Comment at: lib/Parse/ParseDecl.cpp:6254
+ NumElements = ParseConstantExpression(NotTypeCast,
+ /*IsLambdaExprForbidden=*/D.getContext() == D.PrototypeContext);
} else {
I suppose we should forbid these in a lambda parameter declaration clause too?
Repository:
rL LLVM
https://reviews.llvm.org/D28510
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D28510: Reinstate CWG1607 restrictions on lambdas appearing inside certain constant-expressions
faisalv added a comment. Yes - I'll modify the patch to reflect that approach. Any thoughts on whether they should also be forbidden in Lamda parameter declaration clauses? Thanks! Repository: rL LLVM https://reviews.llvm.org/D28510 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D28510: Reinstate CWG1607 restrictions on lambdas appearing inside certain constant-expressions
rsmith added inline comments. Comment at: include/clang/Parse/Parser.h:1432 + ExprResult ParseConstantExpression(TypeCastState isTypeCast = NotTypeCast, + bool IsLambdaExprForbidden = false); ExprResult ParseConstraintExpression(); I would call this `IsInSignature`. Comment at: include/clang/Sema/Sema.h:894-895 +/// \brief Whether lambda expressions are forbidden here. +bool IsLambdaExprForbidden; + Rather than adding a flag, how about we have two different kinds of `ExpressionEvaluationContext` for constant expressions: a generic "constant expression" context and a "constant expression in signature" context? Repository: rL LLVM https://reviews.llvm.org/D28510 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D28510: Reinstate CWG1607 restrictions on lambdas appearing inside certain constant-expressions
faisalv created this revision.
faisalv added a reviewer: rsmith.
faisalv added a subscriber: cfe-commits.
faisalv set the repository for this revision to rL LLVM.
faisalv added a project: clang-c.
This patch disables lambda expressions (especially Immediately Invoked Lambda
Expressions (IILEs, to borrow a term from the all-mighty ecmascript ;)) from
appearing within potentially mangled contexts.
The approach is a rather primitive/inelegant one. Instead of enumerating all
the various syntactic constant-expression contexts at a finer level of
granularity and then passing that information through to each call of
ParseConstantExpression, we simply set a flag that forbids lambda expressions
(when we know we are in the forbidden context). There is probably an
opportunity here to streamline the machinery that prohibits lambdas in certain
contexts across the various versions of C++ further - but that could be
re-engineered if/when Louis Dionne's paper on Lambdas in Unevaluated contexts
gets incorporated into the working draft.
Would appreciate some feedback here - thanks!
Repository:
rL LLVM
https://reviews.llvm.org/D28510
Files:
include/clang/Basic/DiagnosticSemaKinds.td
include/clang/Parse/Parser.h
include/clang/Sema/Sema.h
lib/Parse/ParseDecl.cpp
lib/Parse/ParseExpr.cpp
lib/Parse/ParseTemplate.cpp
lib/Sema/Sema.cpp
lib/Sema/SemaExpr.cpp
test/SemaCXX/cxx1z-constexpr-lambdas.cpp
Index: test/SemaCXX/cxx1z-constexpr-lambdas.cpp
===
--- test/SemaCXX/cxx1z-constexpr-lambdas.cpp
+++ test/SemaCXX/cxx1z-constexpr-lambdas.cpp
@@ -157,6 +157,38 @@
} // end ns1_simple_lambda
+namespace test_forbidden_lambda_expressions {
+
+template struct X { }; //expected-error{{lambda expression may not appear}}
+X<[]{return 10; }()> x; //expected-error{{lambda expression may not appear}}
+void f(int arr[([] { return 5; }())]); //expected-error{{lambda expression may not appear}}
+// FIXME: Should this be ok?
+auto L = [](int arr[([] { return 5; }())]) { }; // OK
+
+// These should be allowed:
+struct A {
+ int : ([] { return 5; }());
+};
+
+int arr[([] { return 5; }())];
+enum { E = [] { return 5; }() };
+static_assert([]{return 5; }() == 5);
+int *ip = new int[([] { return 5; })()];
+
+int test_case(int x) {
+ switch(x) {
+case [] { return 5; }(): //OK
+ break;
+case [] (auto a) { return a; }(6): //expected-note{{previous}}
+ break;
+case 6: //expected-error{{duplicate}}
+ break;
+ }
+ return x;
+}
+
+} // end ns forbidden_lambda_expressions
+
namespace ns1_unimplemented {
namespace ns1_captures {
constexpr auto f(int i) {
Index: lib/Sema/SemaExpr.cpp
===
--- lib/Sema/SemaExpr.cpp
+++ lib/Sema/SemaExpr.cpp
@@ -13083,9 +13083,11 @@
void
Sema::PushExpressionEvaluationContext(ExpressionEvaluationContext NewContext,
Decl *LambdaContextDecl,
- bool IsDecltype) {
+ bool IsDecltype,
+ bool IsLambdaExprForbidden) {
ExprEvalContexts.emplace_back(NewContext, ExprCleanupObjects.size(), Cleanup,
-LambdaContextDecl, IsDecltype);
+LambdaContextDecl, IsDecltype,
+IsLambdaExprForbidden);
Cleanup.reset();
if (!MaybeODRUseExprs.empty())
std::swap(MaybeODRUseExprs, ExprEvalContexts.back().SavedMaybeODRUseExprs);
@@ -13094,9 +13096,11 @@
void
Sema::PushExpressionEvaluationContext(ExpressionEvaluationContext NewContext,
ReuseLambdaContextDecl_t,
- bool IsDecltype) {
+ bool IsDecltype,
+ bool IsLambdaExprForbidden) {
Decl *ClosureContextDecl = ExprEvalContexts.back().ManglingContextDecl;
- PushExpressionEvaluationContext(NewContext, ClosureContextDecl, IsDecltype);
+ PushExpressionEvaluationContext(NewContext, ClosureContextDecl, IsDecltype,
+ IsLambdaExprForbidden);
}
void Sema::PopExpressionEvaluationContext() {
@@ -13117,6 +13121,8 @@
// evaluation of e, following the rules of the abstract machine, would
// evaluate [...] a lambda-expression.
D = diag::err_lambda_in_constant_expression;
+if (getLangOpts().CPlusPlus1z && Rec.IsLambdaExprForbidden)
+ D = diag::err_lambda_in_potentially_mangled_constant_expression;
}
// C++1z allows lambda expressions as core constant expressions.
@@ -13125,9 +13131,11 @@
// are part of function-signatures. Be mindful that P0315 (Lambdas in
// unevaluated contexts) might lift some of these restrictions in a
// future version.
- if (Rec.Context != ConstantEvaluated || !getLangOp
