riccibruno created this revision.
riccibruno added reviewers: bkramer, rjmccall, rsmith, erichkeane.
riccibruno added a project: clang.
Herald added a subscriber: cfe-commits.
riccibruno added a parent revision: D81615: [clang] CWG 2082 and 2346: loosen 
the restrictions on parameters and local variables in default arguments..

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.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D81616

Files:
  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

Index: clang/test/SemaTemplate/instantiate-local-class.cpp
===================================================================
--- clang/test/SemaTemplate/instantiate-local-class.cpp
+++ clang/test/SemaTemplate/instantiate-local-class.cpp
@@ -462,18 +462,15 @@
     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@-1 {{passing argument to parameter 'a' here}}
-      // expected-note@-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@-1 {{passing argument to parameter 'a' here}}
-      // expected-note@-2 {{candidate function not viable}}
-      // expected-note@-3 {{conversion candidate of type}}
-    lambda(); // expected-error {{no matching function}}
+    lambda();
   }
   template void bar<A>(); // expected-note {{in instantiation}}
 }
@@ -494,9 +491,6 @@
   void f(int x = [](T x = nullptr) -> int { return x; }());
   // expected-error@-1 {{cannot initialize a parameter of type 'int' with an rvalue of type 'nullptr_t'}}
   // expected-note@-2 {{passing argument to parameter 'x' here}}
-  // expected-error@-3 {{no matching function for call}}
-  // expected-note@-4 {{candidate function not viable: requires single argument 'x', but no arguments were provided}}
-  // expected-note@-5 {{conversion candidate of type 'auto (*)(int) -> int'}}
 
   void g() { f<int>(); }
   // expected-note@-1 {{in instantiation of default function argument expression for 'f<int>' required here}}
Index: clang/test/SemaCXX/vartemplate-lambda.cpp
===================================================================
--- clang/test/SemaCXX/vartemplate-lambda.cpp
+++ clang/test/SemaCXX/vartemplate-lambda.cpp
@@ -6,10 +6,7 @@
 template<typename T> auto fn1 = [](auto a) { return a + T(1); };
 template<typename T> auto v1 = [](int a = T()) { return a; }();
 // expected-error@-1{{cannot initialize a parameter of type 'int' with an rvalue of type 'int *'}}
-// expected-error@-2{{no matching function for call}}
-// expected-note@-3{{passing argument to parameter 'a' here}}
-// expected-note@-4{{candidate function not viable}}
-// expected-note@-5{{conversion candidate of type 'int (*)(int)'}}
+// expected-note@-2{{passing argument to parameter 'a' here}}
 
 struct S {
   template<class T>
Index: clang/test/CXX/dcl.decl/dcl.meaning/dcl.fct.default/p8.cpp
===================================================================
--- clang/test/CXX/dcl.decl/dcl.meaning/dcl.fct.default/p8.cpp
+++ clang/test/CXX/dcl.decl/dcl.meaning/dcl.fct.default/p8.cpp
@@ -6,5 +6,10 @@
 };
 
 void A::test() {
-  void g(int = this); // expected-error {{default argument references 'this'}}
+  void g(int = this);
+  // expected-error@-1 {{cannot initialize a parameter of type 'int' with an rvalue of type 'A *'}}
+  // expected-note@-2 {{passing argument to parameter here}}
+
+  void h(int = ((void)this,42));
+  // expected-error@-1 {{default argument references 'this'}}
 }
Index: clang/test/CXX/dcl.decl/dcl.meaning/dcl.fct.default/p7.cpp
===================================================================
--- clang/test/CXX/dcl.decl/dcl.meaning/dcl.fct.default/p7.cpp
+++ clang/test/CXX/dcl.decl/dcl.meaning/dcl.fct.default/p7.cpp
@@ -6,8 +6,7 @@
   // expected-error@-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@-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;
Index: clang/lib/Sema/SemaTemplateInstantiate.cpp
===================================================================
--- clang/lib/Sema/SemaTemplateInstantiate.cpp
+++ clang/lib/Sema/SemaTemplateInstantiate.cpp
@@ -2405,7 +2405,12 @@
       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
Index: clang/lib/Sema/SemaDeclCXX.cpp
===================================================================
--- clang/lib/Sema/SemaDeclCXX.cpp
+++ clang/lib/Sema/SemaDeclCXX.cpp
@@ -254,14 +254,12 @@
     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 @@
   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 @@
     // We're done tracking this parameter's instantiations.
     UnparsedDefaultArgInstantiations.erase(InstPos);
   }
-
-  return false;
 }
 
 /// ActOnParamDefaultArgument - Check whether the default argument
@@ -341,13 +342,18 @@
     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
Index: clang/include/clang/Sema/Sema.h
===================================================================
--- clang/include/clang/Sema/Sema.h
+++ clang/include/clang/Sema/Sema.h
@@ -2381,11 +2381,13 @@
   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
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
  • [PATCH] D81616: [clang] Conver... Bruno Ricci via Phabricator via cfe-commits

Reply via email to