[clang] 40ea01f - [clang] Convert a default argument expression to the parameter type...

Bruno Ricci via cfe-commits cfe-commits at lists.llvm.org
Thu Jun 11 05:19:21 PDT 2020


Author: Bruno Ricci
Date: 2020-06-11T13:18:45+01:00
New Revision: 40ea01f6543d0d4aa2701d1b27a6c5413340e7d5

URL: https://github.com/llvm/llvm-project/commit/40ea01f6543d0d4aa2701d1b27a6c5413340e7d5
DIFF: https://github.com/llvm/llvm-project/commit/40ea01f6543d0d4aa2701d1b27a6c5413340e7d5.diff

LOG: [clang] Convert a default argument expression to the parameter type...

...before checking that the default argument is valid with
CheckDefaultArgumentVisitor.

Currently the restrictions on a default argument are checked with the visitor
CheckDefaultArgumentVisitor in ActOnParamDefaultArgument before
performing the conversion to the parameter type in SetParamDefaultArgument.

This was fine before the previous patch but now some valid code post-CWG 2346
is rejected:

void test() {
  const int i2 = 0;
  extern void h2a(int x = i2);     // FIXME: ok, not odr-use
  extern void h2b(int x = i2 + 0); // ok, not odr-use
}

This is because the reference to i2 in h2a has not been marked yet with
NOUR_Constant. i2 is marked NOUR_Constant when the conversion to the parameter
type is done, which is done just after.

The solution is to do the conversion to the parameter type before checking
the restrictions on default arguments with CheckDefaultArgumentVisitor.
This has the side-benefit of improving some diagnostics.

Differential Revision: https://reviews.llvm.org/D81616

Reviewed By: rsmith

Added: 
    

Modified: 
    clang/include/clang/Sema/Sema.h
    clang/lib/Sema/SemaDeclCXX.cpp
    clang/lib/Sema/SemaTemplateInstantiate.cpp
    clang/test/CXX/dcl.decl/dcl.meaning/dcl.fct.default/p7.cpp
    clang/test/CXX/dcl.decl/dcl.meaning/dcl.fct.default/p8.cpp
    clang/test/SemaCXX/vartemplate-lambda.cpp
    clang/test/SemaTemplate/instantiate-local-class.cpp

Removed: 
    


################################################################################
diff  --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h
index 6f074fb941af..3416f339768f 100644
--- a/clang/include/clang/Sema/Sema.h
+++ b/clang/include/clang/Sema/Sema.h
@@ -2381,11 +2381,13 @@ class Sema final {
   void ActOnParamDefaultArgument(Decl *param,
                                  SourceLocation EqualLoc,
                                  Expr *defarg);
-  void ActOnParamUnparsedDefaultArgument(Decl *param,
-                                         SourceLocation EqualLoc,
+  void ActOnParamUnparsedDefaultArgument(Decl *param, SourceLocation EqualLoc,
                                          SourceLocation ArgLoc);
   void ActOnParamDefaultArgumentError(Decl *param, SourceLocation EqualLoc);
-  bool SetParamDefaultArgument(ParmVarDecl *Param, Expr *DefaultArg,
+  ExprResult ConvertParamDefaultArgument(const ParmVarDecl *Param,
+                                         Expr *DefaultArg,
+                                         SourceLocation EqualLoc);
+  void SetParamDefaultArgument(ParmVarDecl *Param, Expr *DefaultArg,
                                SourceLocation EqualLoc);
 
   // Contexts where using non-trivial C union types can be disallowed. This is

diff  --git a/clang/lib/Sema/SemaDeclCXX.cpp b/clang/lib/Sema/SemaDeclCXX.cpp
index 28a84d27c038..a77f7a460242 100644
--- a/clang/lib/Sema/SemaDeclCXX.cpp
+++ b/clang/lib/Sema/SemaDeclCXX.cpp
@@ -254,14 +254,12 @@ void Sema::ImplicitExceptionSpecification::CalledStmt(Stmt *S) {
     ComputedEST = EST_None;
 }
 
-bool
-Sema::SetParamDefaultArgument(ParmVarDecl *Param, Expr *Arg,
-                              SourceLocation EqualLoc) {
+ExprResult Sema::ConvertParamDefaultArgument(const ParmVarDecl *Param,
+                                             Expr *Arg,
+                                             SourceLocation EqualLoc) {
   if (RequireCompleteType(Param->getLocation(), Param->getType(),
-                          diag::err_typecheck_decl_incomplete_type)) {
-    Param->setInvalidDecl();
+                          diag::err_typecheck_decl_incomplete_type))
     return true;
-  }
 
   // C++ [dcl.fct.default]p5
   //   A default argument expression is implicitly converted (clause
@@ -282,7 +280,12 @@ Sema::SetParamDefaultArgument(ParmVarDecl *Param, Expr *Arg,
   CheckCompletedExpr(Arg, EqualLoc);
   Arg = MaybeCreateExprWithCleanups(Arg);
 
-  // Okay: add the default argument to the parameter
+  return Arg;
+}
+
+void Sema::SetParamDefaultArgument(ParmVarDecl *Param, Expr *Arg,
+                                   SourceLocation EqualLoc) {
+  // Add the default argument to the parameter
   Param->setDefaultArg(Arg);
 
   // We have already instantiated this parameter; provide each of the
@@ -296,8 +299,6 @@ Sema::SetParamDefaultArgument(ParmVarDecl *Param, Expr *Arg,
     // We're done tracking this parameter's instantiations.
     UnparsedDefaultArgInstantiations.erase(InstPos);
   }
-
-  return false;
 }
 
 /// ActOnParamDefaultArgument - Check whether the default argument
@@ -341,13 +342,18 @@ Sema::ActOnParamDefaultArgument(Decl *param, SourceLocation EqualLoc,
     return;
   }
 
+  ExprResult Result = ConvertParamDefaultArgument(Param, DefaultArg, EqualLoc);
+  if (Result.isInvalid())
+    return Fail();
+
+  DefaultArg = Result.getAs<Expr>();
+
   // Check that the default argument is well-formed
   CheckDefaultArgumentVisitor DefaultArgChecker(*this, DefaultArg);
   if (DefaultArgChecker.Visit(DefaultArg))
     return Fail();
 
-  if (SetParamDefaultArgument(Param, DefaultArg, EqualLoc))
-    return Fail();
+  SetParamDefaultArgument(Param, DefaultArg, EqualLoc);
 }
 
 /// ActOnParamUnparsedDefaultArgument - We've seen a default

diff  --git a/clang/lib/Sema/SemaTemplateInstantiate.cpp b/clang/lib/Sema/SemaTemplateInstantiate.cpp
index a27b87e3c801..9cbdfcb5f2fb 100644
--- a/clang/lib/Sema/SemaTemplateInstantiate.cpp
+++ b/clang/lib/Sema/SemaTemplateInstantiate.cpp
@@ -2405,7 +2405,12 @@ ParmVarDecl *Sema::SubstParmVarDecl(ParmVarDecl *OldParm,
       if (NewArg.isUsable()) {
         // It would be nice if we still had this.
         SourceLocation EqualLoc = NewArg.get()->getBeginLoc();
-        SetParamDefaultArgument(NewParm, NewArg.get(), EqualLoc);
+        ExprResult Result =
+            ConvertParamDefaultArgument(NewParm, NewArg.get(), EqualLoc);
+        if (Result.isInvalid())
+          return nullptr;
+
+        SetParamDefaultArgument(NewParm, Result.getAs<Expr>(), EqualLoc);
       }
     } else {
       // FIXME: if we non-lazily instantiated non-dependent default args for

diff  --git a/clang/test/CXX/dcl.decl/dcl.meaning/dcl.fct.default/p7.cpp b/clang/test/CXX/dcl.decl/dcl.meaning/dcl.fct.default/p7.cpp
index 07b527bbdc20..af2e7cf09ceb 100644
--- a/clang/test/CXX/dcl.decl/dcl.meaning/dcl.fct.default/p7.cpp
+++ b/clang/test/CXX/dcl.decl/dcl.meaning/dcl.fct.default/p7.cpp
@@ -6,8 +6,7 @@ void h() {
   // expected-error at -1 {{default argument references local variable 'i1' of enclosing function}}
 
   const int i2 = 0;
-  extern void h2a(int x = i2); // FIXME: ok, not odr-use
-  // expected-error at -1 {{default argument references local variable 'i2' of enclosing function}}
+  extern void h2a(int x = i2);     // ok, not odr-use
   extern void h2b(int x = i2 + 0); // ok, not odr-use
 
   const int i3 = 0;

diff  --git a/clang/test/CXX/dcl.decl/dcl.meaning/dcl.fct.default/p8.cpp b/clang/test/CXX/dcl.decl/dcl.meaning/dcl.fct.default/p8.cpp
index ce2c9cb9fb9d..4412f3327539 100644
--- a/clang/test/CXX/dcl.decl/dcl.meaning/dcl.fct.default/p8.cpp
+++ b/clang/test/CXX/dcl.decl/dcl.meaning/dcl.fct.default/p8.cpp
@@ -6,5 +6,10 @@ class A {
 };
 
 void A::test() {
-  void g(int = this); // expected-error {{default argument references 'this'}}
+  void g(int = this);
+  // expected-error at -1 {{cannot initialize a parameter of type 'int' with an rvalue of type 'A *'}}
+  // expected-note at -2 {{passing argument to parameter here}}
+
+  void h(int = ((void)this,42));
+  // expected-error at -1 {{default argument references 'this'}}
 }

diff  --git a/clang/test/SemaCXX/vartemplate-lambda.cpp b/clang/test/SemaCXX/vartemplate-lambda.cpp
index b17c9bb8dcb9..3546193c67d3 100644
--- a/clang/test/SemaCXX/vartemplate-lambda.cpp
+++ b/clang/test/SemaCXX/vartemplate-lambda.cpp
@@ -6,10 +6,7 @@ template <typename> void foo0() { fn0<char>(); }
 template<typename T> auto fn1 = [](auto a) { return a + T(1); };
 template<typename T> auto v1 = [](int a = T()) { return a; }();
 // expected-error at -1{{cannot initialize a parameter of type 'int' with an rvalue of type 'int *'}}
-// expected-error at -2{{no matching function for call}}
-// expected-note at -3{{passing argument to parameter 'a' here}}
-// expected-note at -4{{candidate function not viable}}
-// expected-note at -5{{conversion candidate of type 'int (*)(int)'}}
+// expected-note at -2{{passing argument to parameter 'a' here}}
 
 struct S {
   template<class T>

diff  --git a/clang/test/SemaTemplate/instantiate-local-class.cpp b/clang/test/SemaTemplate/instantiate-local-class.cpp
index e10065c8ea40..15b455f62569 100644
--- a/clang/test/SemaTemplate/instantiate-local-class.cpp
+++ b/clang/test/SemaTemplate/instantiate-local-class.cpp
@@ -462,18 +462,15 @@ namespace rdar23721638 {
     struct Inner { // expected-note {{in instantiation}}
       void operator()(T a = "") {} // expected-error {{conversion function from 'const char [1]' to 'rdar23721638::A' invokes a deleted function}}
       // expected-note at -1 {{passing argument to parameter 'a' here}}
-      // expected-note at -2 {{candidate function not viable}}
     };
-    Inner()(); // expected-error {{no matching function}}
+    Inner()(); // expected-error {{type 'Inner' does not provide a call operator}}
   }
   template void foo<A>(); // expected-note 2 {{in instantiation}}
 
   template <typename T> void bar() {
     auto lambda = [](T a = "") {}; // expected-error {{conversion function from 'const char [1]' to 'rdar23721638::A' invokes a deleted function}}
       // expected-note at -1 {{passing argument to parameter 'a' here}}
-      // expected-note at -2 {{candidate function not viable}}
-      // expected-note at -3 {{conversion candidate of type}}
-    lambda(); // expected-error {{no matching function}}
+    lambda();
   }
   template void bar<A>(); // expected-note {{in instantiation}}
 }
@@ -494,9 +491,6 @@ namespace PR45000 {
   void f(int x = [](T x = nullptr) -> int { return x; }());
   // expected-error at -1 {{cannot initialize a parameter of type 'int' with an rvalue of type 'nullptr_t'}}
   // expected-note at -2 {{passing argument to parameter 'x' here}}
-  // expected-error at -3 {{no matching function for call}}
-  // expected-note at -4 {{candidate function not viable: requires single argument 'x', but no arguments were provided}}
-  // expected-note at -5 {{conversion candidate of type 'auto (*)(int) -> int'}}
 
   void g() { f<int>(); }
   // expected-note at -1 {{in instantiation of default function argument expression for 'f<int>' required here}}


        


More information about the cfe-commits mailing list