]> granicus.if.org Git - clang/commitdiff
Fix PR25627: constant expressions being odr-used in template arguments.
authorFaisal Vali <faisalv@yahoo.com>
Sat, 20 May 2017 19:58:04 +0000 (19:58 +0000)
committerFaisal Vali <faisalv@yahoo.com>
Sat, 20 May 2017 19:58:04 +0000 (19:58 +0000)
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!

git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@303492 91177308-0d34-0410-b5e6-96231b3b80d8

include/clang/Parse/Parser.h
lib/Parse/ParseExpr.cpp
lib/Parse/ParseTemplate.cpp
test/SemaCXX/lambda-expressions.cpp
test/SemaCXX/local-classes.cpp

index c97b4059250b0f60dffac59f196ffe852342240d..537796fa6465d8b7d868f7ad5edf5094a20ad5f7 100644 (file)
@@ -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.
index bcdc72df305ced4161a89a5e894ad3f412046aff..c739a50f0b38b22a99e5225da04f2161365a5811 100644 (file)
@@ -192,6 +192,16 @@ Parser::ParseAssignmentExprWithObjCMessageExprStart(SourceLocation LBracLoc,
   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::ParseConstantExpression(TypeCastState isTypeCast) {
   // 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.
index 28ae113a511ff812da4e9f0fd983a3a6333d0c1b..a919d870810ab5b81f4ad01ea8fb0f87c245cbb0 100644 (file)
@@ -1186,7 +1186,13 @@ ParsedTemplateArgument Parser::ParseTemplateArgument() {
   //   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::ParseTemplateArgument() {
   
   // 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(unsigned Skip) {
 ///         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 {
index e0ab15dc6327476b0a16da28140cae432b8f32fd..efbb47681ade68e5ff28afabd26a1e720e462b18 100644 (file)
@@ -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; };
+  }
+}
index f4ca79159dc9641f419caa96da6c90187719a615..eb0b7e43ebe40fe3789fc950bf56fcbe3421aa4c 100644 (file)
@@ -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
+    }; 
+  }
+}