From 40ea01f6543d0d4aa2701d1b27a6c5413340e7d5 Mon Sep 17 00:00:00 2001 From: Bruno Ricci Date: Thu, 11 Jun 2020 13:13:05 +0100 Subject: [PATCH] [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 --- clang/include/clang/Sema/Sema.h | 8 ++++--- clang/lib/Sema/SemaDeclCXX.cpp | 28 +++++++++++++--------- clang/lib/Sema/SemaTemplateInstantiate.cpp | 7 +++++- .../dcl.decl/dcl.meaning/dcl.fct.default/p7.cpp | 3 +-- .../dcl.decl/dcl.meaning/dcl.fct.default/p8.cpp | 7 +++++- clang/test/SemaCXX/vartemplate-lambda.cpp | 5 +--- .../test/SemaTemplate/instantiate-local-class.cpp | 10 ++------ 7 files changed, 38 insertions(+), 30 deletions(-) diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h index 6f074fb..3416f33 100644 --- a/clang/include/clang/Sema/Sema.h +++ b/clang/include/clang/Sema/Sema.h @@ -2381,11 +2381,13 @@ public: 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 28a84d2..a77f7a4 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(); + // 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 a27b87e..9cbdfcb 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(), 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 07b527b..af2e7cf 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@-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; 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 ce2c9cb..4412f33 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@-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'}} } diff --git a/clang/test/SemaCXX/vartemplate-lambda.cpp b/clang/test/SemaCXX/vartemplate-lambda.cpp index b17c9bb..3546193 100644 --- a/clang/test/SemaCXX/vartemplate-lambda.cpp +++ b/clang/test/SemaCXX/vartemplate-lambda.cpp @@ -6,10 +6,7 @@ template void foo0() { fn0(); } template auto fn1 = [](auto a) { return a + T(1); }; template 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 diff --git a/clang/test/SemaTemplate/instantiate-local-class.cpp b/clang/test/SemaTemplate/instantiate-local-class.cpp index e10065c..15b455f 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@-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(); // expected-note 2 {{in instantiation}} template 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(); // expected-note {{in instantiation}} } @@ -494,9 +491,6 @@ namespace PR45000 { 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(); } // expected-note@-1 {{in instantiation of default function argument expression for 'f' required here}} -- 2.7.4