Author: faisalv
Date: Sat May 20 14:58:04 2017
New Revision: 303492
URL: http://llvm.org/viewvc/llvm-project?rev=303492&view=rev
Log:
Fix PR25627: constant expressions being odr-used in template arguments.
This patch ensures that clang processes the expression-nodes that are generated
when disambiguating between types and expressions within template arguments as
constant-expressions by installing the ConstantEvaluated
ExpressionEvaluationContext just before attempting the disambiguation - and
then making sure that Context carries through into ParseConstantExpression (by
refactoring it out into a function that does not create its own
EvaluationContext: ParseConstantExpressionInExprEvalContext)
Note, prior to this patch, trunk would correctly disambiguate and identify the
expression as an expression - and while it would annotate the token with the
expression - it would fail to complete the odr-use processing (specifically,
failing to trigger Sema::UpdateMarkingForLValueToRValue as is done for all
Constant Expressions, which would remove it from being considered odr-used).
By installing the ConstantExpression Evaluation Context prior to
disambiguation, and making sure it carries though, we ensure correct processing
of the expression-node.
For e.g:
template<int> struct X { };
void f() {
const int N = 10;
X<N> x; // should be OK.
[] { return X<N>{}; }; // Should be OK - no capture - but clang errors!
}
See a related bug: https://bugs.llvm.org//show_bug.cgi?id=25627
In summary (and reiteration), the fix is as follows:
- Remove the EnteredConstantEvaluatedContext action from
ParseTemplateArgumentList (relying on ParseTemplateArgument getting it right)
- Add the EnteredConstantEvaluatedContext action just prior to undergoing
the disambiguating parse, and if the parse succeeds for an expression, carry
the context though into a refactored version of ParseConstantExpression that
does not create its own ExpressionEvaluationContext.
See https://reviews.llvm.org/D31588 for additional context regarding some of
the more fragile and complicated approaches attempted, and Richard's feedback
that eventually shaped the simpler and more robust rendition that is being
committed.
Thanks Richard!
Modified:
cfe/trunk/include/clang/Parse/Parser.h
cfe/trunk/lib/Parse/ParseExpr.cpp
cfe/trunk/lib/Parse/ParseTemplate.cpp
cfe/trunk/test/SemaCXX/lambda-expressions.cpp
cfe/trunk/test/SemaCXX/local-classes.cpp
Modified: cfe/trunk/include/clang/Parse/Parser.h
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Parse/Parser.h?rev=303492&r1=303491&r2=303492&view=diff
==============================================================================
--- cfe/trunk/include/clang/Parse/Parser.h (original)
+++ cfe/trunk/include/clang/Parse/Parser.h Sat May 20 14:58:04 2017
@@ -1460,6 +1460,7 @@ public:
};
ExprResult ParseExpression(TypeCastState isTypeCast = NotTypeCast);
+ ExprResult ParseConstantExpressionInExprEvalContext(TypeCastState
isTypeCast);
ExprResult ParseConstantExpression(TypeCastState isTypeCast = NotTypeCast);
ExprResult ParseConstraintExpression();
// Expr that doesn't include commas.
Modified: cfe/trunk/lib/Parse/ParseExpr.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Parse/ParseExpr.cpp?rev=303492&r1=303491&r2=303492&view=diff
==============================================================================
--- cfe/trunk/lib/Parse/ParseExpr.cpp (original)
+++ cfe/trunk/lib/Parse/ParseExpr.cpp Sat May 20 14:58:04 2017
@@ -192,6 +192,16 @@ Parser::ParseAssignmentExprWithObjCMessa
return ParseRHSOfBinaryExpression(R, prec::Assignment);
}
+ExprResult
+Parser::ParseConstantExpressionInExprEvalContext(TypeCastState isTypeCast) {
+ assert(Actions.ExprEvalContexts.back().Context ==
+ Sema::ExpressionEvaluationContext::ConstantEvaluated &&
+ "Call this function only if your ExpressionEvaluationContext is "
+ "already ConstantEvaluated");
+ ExprResult LHS(ParseCastExpression(false, false, isTypeCast));
+ ExprResult Res(ParseRHSOfBinaryExpression(LHS, prec::Conditional));
+ return Actions.ActOnConstantExpression(Res);
+}
ExprResult Parser::ParseConstantExpression(TypeCastState isTypeCast) {
// C++03 [basic.def.odr]p2:
@@ -200,10 +210,7 @@ ExprResult Parser::ParseConstantExpressi
// C++98 and C++11 have no such rule, but this is only a defect in C++98.
EnterExpressionEvaluationContext ConstantEvaluated(
Actions, Sema::ExpressionEvaluationContext::ConstantEvaluated);
-
- ExprResult LHS(ParseCastExpression(false, false, isTypeCast));
- ExprResult Res(ParseRHSOfBinaryExpression(LHS, prec::Conditional));
- return Actions.ActOnConstantExpression(Res);
+ return ParseConstantExpressionInExprEvalContext(isTypeCast);
}
/// \brief Parse a constraint-expression.
Modified: cfe/trunk/lib/Parse/ParseTemplate.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Parse/ParseTemplate.cpp?rev=303492&r1=303491&r2=303492&view=diff
==============================================================================
--- cfe/trunk/lib/Parse/ParseTemplate.cpp (original)
+++ cfe/trunk/lib/Parse/ParseTemplate.cpp Sat May 20 14:58:04 2017
@@ -1186,7 +1186,13 @@ ParsedTemplateArgument Parser::ParseTemp
// expression is resolved to a type-id, regardless of the form of
// the corresponding template-parameter.
//
- // Therefore, we initially try to parse a type-id.
+ // Therefore, we initially try to parse a type-id - and isCXXTypeId might
look
+ // up and annotate an identifier as an id-expression during disambiguation,
+ // so enter the appropriate context for a constant expression template
+ // argument before trying to disambiguate.
+
+ EnterExpressionEvaluationContext EnterConstantEvaluated(
+ Actions, Sema::ExpressionEvaluationContext::ConstantEvaluated);
if (isCXXTypeId(TypeIdAsTemplateArgument)) {
SourceLocation Loc = Tok.getLocation();
TypeResult TypeArg = ParseTypeName(/*Range=*/nullptr,
@@ -1216,7 +1222,7 @@ ParsedTemplateArgument Parser::ParseTemp
// Parse a non-type template argument.
SourceLocation Loc = Tok.getLocation();
- ExprResult ExprArg = ParseConstantExpression(MaybeTypeCast);
+ ExprResult ExprArg = ParseConstantExpressionInExprEvalContext(MaybeTypeCast);
if (ExprArg.isInvalid() || !ExprArg.get())
return ParsedTemplateArgument();
@@ -1262,9 +1268,7 @@ bool Parser::IsTemplateArgumentList(unsi
/// template-argument-list ',' template-argument
bool
Parser::ParseTemplateArgumentList(TemplateArgList &TemplateArgs) {
- // Template argument lists are constant-evaluation contexts.
- EnterExpressionEvaluationContext EvalContext(
- Actions, Sema::ExpressionEvaluationContext::ConstantEvaluated);
+
ColonProtectionRAIIObject ColonProtection(*this, false);
do {
Modified: cfe/trunk/test/SemaCXX/lambda-expressions.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/lambda-expressions.cpp?rev=303492&r1=303491&r2=303492&view=diff
==============================================================================
--- cfe/trunk/test/SemaCXX/lambda-expressions.cpp (original)
+++ cfe/trunk/test/SemaCXX/lambda-expressions.cpp Sat May 20 14:58:04 2017
@@ -525,9 +525,9 @@ template <int N>
class S {};
void foo() {
- const int num = 18; // expected-note {{'num' declared here}}
+ const int num = 18;
auto outer = []() {
- auto inner = [](S<num> &X) {}; // expected-error {{variable 'num' cannot
be implicitly captured in a lambda with no capture-default specified}}
+ auto inner = [](S<num> &X) {};
};
}
}
@@ -573,3 +573,13 @@ void foo1() {
auto s1 = S1{[name=name]() {}}; // expected-error {{use of undeclared
identifier 'name'; did you mean 'name1'?}}
}
}
+
+namespace PR25627_dont_odr_use_local_consts {
+
+ template<int> struct X {};
+
+ void foo() {
+ const int N = 10;
+ (void) [] { X<N> x; };
+ }
+}
Modified: cfe/trunk/test/SemaCXX/local-classes.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/local-classes.cpp?rev=303492&r1=303491&r2=303492&view=diff
==============================================================================
--- cfe/trunk/test/SemaCXX/local-classes.cpp (original)
+++ cfe/trunk/test/SemaCXX/local-classes.cpp Sat May 20 14:58:04 2017
@@ -40,3 +40,15 @@ namespace Templates {
};
}
}
+
+namespace PR25627_dont_odr_use_local_consts {
+ template<int> struct X { X(); X(int); };
+
+ void foo() {
+ const int N = 10;
+
+ struct Local {
+ void f(X<N> = X<N>()) {} // OK
+ };
+ }
+}
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
