ahatanak updated this revision to Diff 114753. ahatanak added a comment. Address review comments.
- Detect invalid references to parameters or local variables by default arguments in tryCaptureVariable. Before parsing or instantiating the default argument expression, the enclosing DeclContext is saved to ParentOfDefaultArg, which tryCaptureVariable uses to detect invalid references (if the referenced variable belongs to ParentOfDefaultArg or an enclosing DeclContext, it is not valid). - In CheckCXXDefaultArgExpr, save the parameters and their instantiations that appear before the parameter with default argument to the current LocalInstantiationScope so that findInstantiationOf doesn't assert when it tries to find the instantiation of a parameter that is referenced in the default arugment. There are still cases where clang rejects references to local variables or parameters that shouldn't be rejected. For example: - Local variables or parameters referenced in _Generic's controlling-expression or the expressions of the selections that are not chosen. - The following code is rejected even though 'x' is not odr-used: void func() { const int x = 1; void foo1(int a0 = x); } - dcl.fct.default/p7.cpp I plan to work on a fix after this patch is committed. https://reviews.llvm.org/D36915 Files: include/clang/Sema/Sema.h lib/Parse/ParseDecl.cpp lib/Sema/Sema.cpp lib/Sema/SemaExpr.cpp test/CXX/dcl.decl/dcl.meaning/dcl.fct.default/p7.cpp test/CXX/expr/expr.prim/expr.prim.lambda/p13.cpp test/SemaCXX/default1.cpp test/SemaObjCXX/blocks.mm
Index: test/SemaObjCXX/blocks.mm =================================================================== --- test/SemaObjCXX/blocks.mm +++ test/SemaObjCXX/blocks.mm @@ -169,3 +169,17 @@ return b; // expected-error {{no viable conversion from returned value of type 'MoveBlockVariable::B0' to function return type 'MoveBlockVariable::B1'}} } } + +namespace DefaultArg { +void test() { + id x; + void func0(id a0, id a1 = ^{ (void)&a0; }); // expected-error {{default argument references parameter 'a0'}} + void func1(id a0, id a1 = ^{ (void)&x; }); // expected-error {{default argument references local variable 'x' of enclosing function}} + void func2(id a0, id a1 = ^{ (void)sizeof(a0); }); + void func3(id a0 = ^{ (void)sizeof(x); }); + void func4(id a0, id a1 = ^{ + ^{ (void)&a0; }(); // expected-error {{default argument references parameter 'a0'}} + [=](){ (void)&a0; }(); // expected-error {{default argument references parameter 'a0'}} + }); +} +} Index: test/SemaCXX/default1.cpp =================================================================== --- test/SemaCXX/default1.cpp +++ test/SemaCXX/default1.cpp @@ -78,3 +78,79 @@ void PR20769_b(int = 1); void PR20769_b() { void PR20769_b(int = 2); } + +#if __cplusplus >= 201103L // C++11 or later +struct S2 { + template<class T> + S2(T&&) {} +}; + +template<class T> +void func0(int a0, S2 a1 = [](){ (void)&a0; }); // expected-error {{default argument references parameter 'a0'}} + +// FIXME: There shouldn't be any warnings about variables referenced in the +// controlling-expression or the expressions of the selections that are +// not chosen. +template<class T> +void func1(T a0, int a1, S2 a2 = _Generic((a0), default: [](){ (void)&a1; }, int: 0)); // expected-error {{default argument references parameter 'a0'}} + +template<class T> +void func2(S2 a0 = [](){ + int t; [&t](){ (void)&t;}(); +}); + +template<class T> +void func3(int a0, S2 a1 = [](){ + [=](){ (void)&a0;}(); // expected-error {{default argument references parameter 'a0'}} +}); + +void func4(int, int); + +template<class...Ts> +void func5(Ts...a0, S2 a1 = [](){ func4(a0...); }) { // expected-error 2 {{default argument references parameter 'a0'}} +} + +template<class...Ts> +void func6(Ts...a0, S2 a1 = [](){ (void)sizeof...(a0); }) { +} + +double d; + +void test1() { + int i; + float f; + // FIXME: There shouldn't be any warnings about variables referenced in the + // controlling-expression or the expressions of the selections that are + // not chosen. + void foo0(int a0 = _Generic((f), double: d, float: f)); // expected-error 2 {{default argument references local variable 'f' of enclosing function}} + void foo1(int a0 = _Generic((d), double: d, float: f)); // expected-error {{default argument references local variable 'f' of enclosing function}} + void foo2(int a0 = _Generic((i), int: d, float: f)); // expected-error {{default argument references local variable 'i' of enclosing function}} expected-error {{default argument references local variable 'f' of enclosing function}} + void foo3(int a0 = _Generic((i), default: d, float: f)); // expected-error {{default argument references local variable 'i' of enclosing function}} expected-error {{default argument references local variable 'f' of enclosing function}} + + void foo4(S2 a0 = [&](){ (void)&i; }); // expected-error {{default argument references local variable 'i' of enclosing function}} + void foo5(S2 a0 = [](){ + // No warning about capture list of a lambda expression defined in a block scope. + int t; [&t](){ (void)&t;}(); + }); + void foo6(int a0, S2 a1 = [](){ + // No warning about local variables or parameters referenced by an + // unevaluated expression. + int t = sizeof({i, a0, a1;}); + }); + void foo6(S2 a0 = [](){ + int i; + void foo7(int a0, + S2 a1 = [](){ (void)&a0; }); // expected-error {{default argument references parameter 'a0'}} + void foo8(S2 a0 = [](){ (void)&i; }); // expected-error {{default argument references local variable 'i' of enclosing function}} + }); + + func0<int>(1); // expected-note {{in instantiation of default function argument expression for 'func0<int>' required here}} + func1<int>(1); // expected-error {{no matching function for call to 'func1'}} + func2<int>(); + func3<int>(1); // expected-note {{in instantiation of default function argument expression for 'func3<int>' required}} + func5<int, int>(1, 2); // expected-note {{in instantiation of default function argument expression for 'func5<int, int>' required here}} + func6<int, int>(1, 2); + +} + +#endif Index: test/CXX/expr/expr.prim/expr.prim.lambda/p13.cpp =================================================================== --- test/CXX/expr/expr.prim/expr.prim.lambda/p13.cpp +++ test/CXX/expr/expr.prim/expr.prim.lambda/p13.cpp @@ -2,9 +2,9 @@ void f2() { int i = 1; - void g1(int = ([i]{ return i; })()); // expected-error{{lambda expression in default argument cannot capture any entity}} - void g2(int = ([i]{ return 0; })()); // expected-error{{lambda expression in default argument cannot capture any entity}} - void g3(int = ([=]{ return i; })()); // expected-error{{lambda expression in default argument cannot capture any entity}} + void g1(int = ([i]{ return i; })()); // expected-error 2{{default argument references local variable 'i' of enclosing function}} + void g2(int = ([i]{ return 0; })()); // expected-error{{default argument references local variable 'i' of enclosing function}} + void g3(int = ([=]{ return i; })()); // expected-error{{default argument references local variable 'i' of enclosing function}} void g4(int = ([=]{ return 0; })()); void g5(int = ([]{ return sizeof i; })()); } Index: test/CXX/dcl.decl/dcl.meaning/dcl.fct.default/p7.cpp =================================================================== --- test/CXX/dcl.decl/dcl.meaning/dcl.fct.default/p7.cpp +++ test/CXX/dcl.decl/dcl.meaning/dcl.fct.default/p7.cpp @@ -3,5 +3,7 @@ void h() { int i; + // FIXME: There should be no warnings here. 'i' is referenced in an unevaluated + // expression. extern void h2(int x = sizeof(i)); // expected-error {{default argument references local variable 'i' of enclosing function}} } Index: lib/Sema/SemaExpr.cpp =================================================================== --- lib/Sema/SemaExpr.cpp +++ lib/Sema/SemaExpr.cpp @@ -2746,8 +2746,8 @@ } static void -diagnoseUncapturableValueReference(Sema &S, SourceLocation loc, - ValueDecl *var, DeclContext *DC); +diagnoseUncapturableValueReference(Sema &S, SourceLocation loc, ValueDecl *var, + bool InDefaultArgExpr = false); /// \brief Complete semantic analysis for a reference to the given declaration. ExprResult Sema::BuildDeclarationNameExpr( @@ -2905,7 +2905,7 @@ auto *BD = cast<BindingDecl>(VD); if (BD->getDeclContext()->isFunctionOrMethod() && BD->getDeclContext() != CurContext) - diagnoseUncapturableValueReference(*this, Loc, BD, CurContext); + diagnoseUncapturableValueReference(*this, Loc, BD); break; } @@ -4514,7 +4514,31 @@ // the semantic constraints are checked, at the point where the // default argument expression appears. ContextRAII SavedContext(*this, FD); + DefaultArgRAII DefaultArgContext(*this); LocalInstantiationScope Local(*this); + + // Add mappings for instantiated parameters appearing before Param. This + // is needed to instantiate default argument expressions referencing + // other parameters in unevaluated contexts. + if (FunctionDecl *Pattern = FD->getTemplateInstantiationPattern()) { + auto I = FD->param_begin(); + for (const auto *PVD : Pattern->parameters()) { + if (*I == Param) + break; + + if (!PVD->isParameterPack()) { + CurrentInstantiationScope->InstantiatedLocal(PVD, *I++); + continue; + } + + unsigned NumExpansions = + *getNumArgumentsInExpansion(PVD->getType(), MutiLevelArgList); + CurrentInstantiationScope->MakeInstantiatedLocalArgPack(PVD); + while (NumExpansions--) + CurrentInstantiationScope->InstantiatedLocalPackArg(PVD, *I++); + } + } + Result = SubstInitializer(UninstExpr, MutiLevelArgList, /*DirectInit*/false); } @@ -13687,13 +13711,13 @@ static void diagnoseUncapturableValueReference(Sema &S, SourceLocation loc, - ValueDecl *var, DeclContext *DC) { + ValueDecl *var, bool InDefaultArgExpr) { DeclContext *VarDC = var->getDeclContext(); // If the parameter still belongs to the translation unit, then // we're actually just using one parameter in the declaration of // the next. - if (isa<ParmVarDecl>(var) && + if (!InDefaultArgExpr && isa<ParmVarDecl>(var) && isa<TranslationUnitDecl>(VarDC)) return; @@ -13718,10 +13742,19 @@ ContextKind = 1; } - S.Diag(loc, diag::err_reference_to_local_in_enclosing_context) - << var << ValueKind << ContextKind << VarDC; - S.Diag(var->getLocation(), diag::note_entity_declared_at) - << var; + if (InDefaultArgExpr) { + unsigned DiagID; + if (isa<ParmVarDecl>(var)) + DiagID = diag::err_param_default_argument_references_param; + else + DiagID = diag::err_param_default_argument_references_local; + S.Diag(loc, DiagID) << var->getDeclName(); + } else { + S.Diag(loc, diag::err_reference_to_local_in_enclosing_context) + << var << ValueKind << ContextKind << VarDC; + S.Diag(var->getLocation(), diag::note_entity_declared_at) + << var; + } // FIXME: Add additional diagnostic info about class etc. which prevents // capture. @@ -13757,16 +13790,28 @@ return false; } +static bool invalidLocalVarReferenceInDefaultArg(VarDecl *Var, DeclContext *DC, + Sema &S) { + return Var->hasLocalStorage() && DC == S.getParentOfDefaultArg(); +} + // Only block literals, captured statements, and lambda expressions can // capture; other scopes don't work. static DeclContext *getParentOfCapturingContextOrNull(DeclContext *DC, VarDecl *Var, SourceLocation Loc, const bool Diagnose, Sema &S) { - if (isa<BlockDecl>(DC) || isa<CapturedDecl>(DC) || isLambdaCallOperator(DC)) - return getLambdaAwareParentOfDeclContext(DC); - else if (Var->hasLocalStorage()) { + bool InDefaultArgExpr = false; + + if (isa<BlockDecl>(DC) || isa<CapturedDecl>(DC) || isLambdaCallOperator(DC)) { + DeclContext *ParentContext = getLambdaAwareParentOfDeclContext(DC); + if (!invalidLocalVarReferenceInDefaultArg(Var, ParentContext, S)) + return ParentContext; + InDefaultArgExpr = true; + } + + if (Var->hasLocalStorage()) { if (Diagnose) - diagnoseUncapturableValueReference(S, Loc, Var, DC); + diagnoseUncapturableValueReference(S, Loc, Var, InDefaultArgExpr); } return nullptr; } @@ -14192,7 +14237,14 @@ // If the variable is declared in the current context, there is no need to // capture it. - if (VarDC == DC) return true; + if (VarDC == DC) { + // Emit diagnositics if Var is a local variable defined in a DeclContext + // enclosing a default argument. + if (BuildAndDiagnose && + invalidLocalVarReferenceInDefaultArg(Var, DC, *this)) + diagnoseUncapturableValueReference(*this, ExprLoc, Var, true); + return true; + } // Capture global variables if it is required to use private copy of this // variable. @@ -14255,7 +14307,7 @@ << Var->getDeclName(); Diag(LSI->Lambda->getLocStart(), diag::note_lambda_decl); } else - diagnoseUncapturableValueReference(*this, ExprLoc, Var, DC); + diagnoseUncapturableValueReference(*this, ExprLoc, Var); } return true; } Index: lib/Sema/Sema.cpp =================================================================== --- lib/Sema/Sema.cpp +++ lib/Sema/Sema.cpp @@ -1674,6 +1674,15 @@ return Ident___float128; } +Sema::DefaultArgRAII::DefaultArgRAII(Sema &S) + : Actions(S), PrevParent(S.getParentOfDefaultArg()) { + S.setParentOfDefaultArg(S.CurContext); +} + +Sema::DefaultArgRAII::~DefaultArgRAII() { + Actions.setParentOfDefaultArg(PrevParent); +} + void Sema::PushCapturedRegionScope(Scope *S, CapturedDecl *CD, RecordDecl *RD, CapturedRegionKind K) { CapturingScopeInfo *CSI = new CapturedRegionScopeInfo( Index: lib/Parse/ParseDecl.cpp =================================================================== --- lib/Parse/ParseDecl.cpp +++ lib/Parse/ParseDecl.cpp @@ -6313,6 +6313,8 @@ if (Tok.is(tok::equal)) { SourceLocation EqualLoc = Tok.getLocation(); + Sema::DefaultArgRAII DefaultArgContext(Actions); + // Parse the default argument if (D.getContext() == Declarator::MemberContext) { // If we're inside a class definition, cache the tokens Index: include/clang/Sema/Sema.h =================================================================== --- include/clang/Sema/Sema.h +++ include/clang/Sema/Sema.h @@ -1048,6 +1048,37 @@ /// for C++ records. llvm::FoldingSet<SpecialMemberOverloadResultEntry> SpecialMemberCache; + /// Parent DeclContext of the default argument expression that is currently + /// being parsed. This is used to detect invalid local variable references in + /// default argument expression. For example: + /// + /// void foo1() { + /// int i; + /// + /// // references to 'i' and 'a0' are invalid. + /// void foo2(int a0, int a1 = i, int a2 = a0); + /// } + + const DeclContext *ParentOfDefaultArg = nullptr; + + void setParentOfDefaultArg(const DeclContext *P) { + ParentOfDefaultArg = P; + } + + const DeclContext *getParentOfDefaultArg() const { + return ParentOfDefaultArg; + } + + /// RAII class to save and restore ParentOfDefaultArg. + class DefaultArgRAII { + public: + DefaultArgRAII(Sema &S); + ~DefaultArgRAII(); + private: + Sema &Actions; + const DeclContext *PrevParent; + }; + /// \brief A cache of the flags available in enumerations with the flag_bits /// attribute. mutable llvm::DenseMap<const EnumDecl*, llvm::APInt> FlagBitsCache;
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits