From: Douglas Gregor Date: Wed, 1 Feb 2012 01:18:43 +0000 (+0000) Subject: Improve checking of explicit captures in a C++11 lambda expression: X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=93962e5360a43200faa70939571afc4fb9326cf7;p=clang Improve checking of explicit captures in a C++11 lambda expression: - Actually building the var -> capture mapping properly (there was an off-by-one error) - Keeping track of the source location of each capture - Minor QoI improvements, e.g, highlighing the prior capture if there are multiple captures, pointing at the variable declaration we found if we reject it. As part of this, add standard citations for the various semantic checks we perform, and note where we're not performing those checks as we should. git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@149462 91177308-0d34-0410-b5e6-96231b3b80d8 --- diff --git a/include/clang/Sema/ScopeInfo.h b/include/clang/Sema/ScopeInfo.h index 7ffc3bcfb9..bff2cf8ad2 100644 --- a/include/clang/Sema/ScopeInfo.h +++ b/include/clang/Sema/ScopeInfo.h @@ -140,15 +140,18 @@ public: // copy constructor. llvm::PointerIntPair CopyExprAndNested; + /// \brief The source location at which the first capture occurred.. + SourceLocation Loc; + public: - Capture(VarDecl *Var, bool isByref, bool isNested, Expr *Cpy) + Capture(VarDecl *Var, bool isByref, bool isNested, SourceLocation Loc, + Expr *Cpy) : VarAndKind(Var, isByref ? Cap_ByRef : Cap_ByVal), CopyExprAndNested(Cpy, isNested) {} enum IsThisCapture { ThisCapture }; - Capture(IsThisCapture, bool isNested) - : VarAndKind(0, Cap_This), - CopyExprAndNested(0, isNested) { + Capture(IsThisCapture, bool isNested, SourceLocation Loc) + : VarAndKind(0, Cap_This), CopyExprAndNested(0, isNested), Loc(Loc) { } bool isThisCapture() const { return VarAndKind.getInt() == Cap_This; } @@ -160,6 +163,10 @@ public: VarDecl *getVariable() const { return VarAndKind.getPointer(); } + + /// \brief Retrieve the location at which this variable was captured. + SourceLocation getLocation() const { return Loc; } + Expr *getCopyExpr() const { return CopyExprAndNested.getPointer(); } @@ -188,13 +195,14 @@ public: /// or null if unknown. QualType ReturnType; - void AddCapture(VarDecl *Var, bool isByref, bool isNested, Expr *Cpy) { - Captures.push_back(Capture(Var, isByref, isNested, Cpy)); + void AddCapture(VarDecl *Var, bool isByref, bool isNested, SourceLocation Loc, + Expr *Cpy) { + Captures.push_back(Capture(Var, isByref, isNested, Loc, Cpy)); CaptureMap[Var] = Captures.size(); } - void AddThisCapture(bool isNested) { - Captures.push_back(Capture(Capture::ThisCapture, isNested)); + void AddThisCapture(bool isNested, SourceLocation Loc) { + Captures.push_back(Capture(Capture::ThisCapture, isNested, Loc)); CXXThisCaptureIndex = Captures.size(); } diff --git a/lib/Sema/SemaExpr.cpp b/lib/Sema/SemaExpr.cpp index 0d929f1836..5de0388c06 100644 --- a/lib/Sema/SemaExpr.cpp +++ b/lib/Sema/SemaExpr.cpp @@ -1263,7 +1263,8 @@ static CaptureResult propagateCapture(Sema &S, unsigned ValidScopeIndex, i != e; ++i) { BlockScopeInfo *innerBlock = cast(S.FunctionScopes[i]); innerBlock->AddCapture(Cap.getVariable(), Cap.isReferenceCapture(), - /*nested*/ true, Cap.getCopyExpr()); + /*nested*/ true, Cap.getLocation(), + Cap.getCopyExpr()); } return Cap.isReferenceCapture() ? CR_CaptureByRef : CR_Capture; @@ -1377,7 +1378,7 @@ static CaptureResult shouldCaptureValueReference(Sema &S, SourceLocation loc, cast(S.FunctionScopes[functionScopesIndex]); // Build a valid capture in this scope. - blockScope->AddCapture(var, byRef, /*nested*/ false, copyExpr); + blockScope->AddCapture(var, byRef, /*nested*/ false, loc, copyExpr); // Propagate that to inner captures if necessary. return propagateCapture(S, functionScopesIndex, diff --git a/lib/Sema/SemaExprCXX.cpp b/lib/Sema/SemaExprCXX.cpp index a24063f860..0a987511fb 100644 --- a/lib/Sema/SemaExprCXX.cpp +++ b/lib/Sema/SemaExprCXX.cpp @@ -706,7 +706,7 @@ void Sema::CheckCXXThisCapture(SourceLocation Loc) { NumClosures; --idx, --NumClosures) { CapturingScopeInfo *CSI = cast(FunctionScopes[idx]); bool isNested = NumClosures > 1; - CSI->AddThisCapture(isNested); + CSI->AddThisCapture(isNested, Loc); } } @@ -4869,16 +4869,27 @@ void Sema::ActOnStartOfLambdaDefinition(LambdaIntroducer &Intro, for (llvm::SmallVector::const_iterator C = Intro.Captures.begin(), E = Intro.Captures.end(); C != E; ++C) { if (C->Kind == LCK_This) { + // C++11 [expr.prim.lambda]p8: + // An identifier or this shall not appear more than once in a + // lambda-capture. if (!ThisCaptureType.isNull()) { - Diag(C->Loc, diag::err_capture_more_than_once) << "'this'"; + Diag(C->Loc, diag::err_capture_more_than_once) + << "'this'" + << SourceRange(Captures[CXXThisCaptureIndex].getLocation()); continue; } + // C++11 [expr.prim.lambda]p8: + // If a lambda-capture includes a capture-default that is =, the + // lambda-capture shall not contain this [...]. if (Intro.Default == LCD_ByCopy) { Diag(C->Loc, diag::err_this_capture_with_copy_default); continue; } + // C++11 [expr.prim.lambda]p12: + // If this is captured by a local lambda expression, its nearest + // enclosing function shall be a non-static member function. ThisCaptureType = getCurrentThisType(); if (ThisCaptureType.isNull()) { Diag(C->Loc, diag::err_invalid_this_use); @@ -4888,15 +4899,20 @@ void Sema::ActOnStartOfLambdaDefinition(LambdaIntroducer &Intro, // FIXME: Need getCurCapture(). bool isNested = getCurBlock() || getCurLambda(); + CXXThisCaptureIndex = Captures.size(); CapturingScopeInfo::Capture Cap(CapturingScopeInfo::Capture::ThisCapture, - isNested); + isNested, C->Loc); Captures.push_back(Cap); - CXXThisCaptureIndex = Captures.size(); continue; } assert(C->Id && "missing identifier for capture"); + // C++11 [expr.prim.lambda]p8: + // If a lambda-capture includes a capture-default that is &, the + // identifiers in the lambda-capture shall not be preceded by &. + // If a lambda-capture includes a capture-default that is =, [...] + // each identifier it contains shall be preceded by &. if (C->Kind == LCK_ByRef && Intro.Default == LCD_ByRef) { Diag(C->Loc, diag::err_reference_capture_with_reference_default); continue; @@ -4907,43 +4923,56 @@ void Sema::ActOnStartOfLambdaDefinition(LambdaIntroducer &Intro, DeclarationNameInfo Name(C->Id, C->Loc); LookupResult R(*this, Name, LookupOrdinaryName); - CXXScopeSpec ScopeSpec; - LookupParsedName(R, CurScope, &ScopeSpec); + LookupName(R, CurScope); if (R.isAmbiguous()) continue; if (R.empty()) { + // FIXME: Disable corrections that would add qualification? + CXXScopeSpec ScopeSpec; DeclFilterCCC Validator; if (DiagnoseEmptyLookup(CurScope, ScopeSpec, R, Validator)) continue; } + // C++11 [expr.prim.lambda]p10: + // The identifiers in a capture-list are looked up using the usual rules + // for unqualified name lookup (3.4.1); each such lookup shall find a + // variable with automatic storage duration declared in the reaching + // scope of the local lambda expression. + // FIXME: Check reaching scope. VarDecl *Var = R.getAsSingle(); if (!Var) { Diag(C->Loc, diag::err_capture_does_not_name_variable) << C->Id; continue; } - if (CaptureMap.count(Var)) { - Diag(C->Loc, diag::err_capture_more_than_once) << C->Id; - continue; - } - if (!Var->hasLocalStorage()) { Diag(C->Loc, diag::err_capture_non_automatic_variable) << C->Id; + Diag(Var->getLocation(), diag::note_previous_decl) << C->Id; continue; } - + if (Var->hasAttr()) { Diag(C->Loc, diag::err_lambda_capture_block) << C->Id; Diag(Var->getLocation(), diag::note_previous_decl) << C->Id; continue; } + + // C++11 [expr.prim.lambda]p8: + // An identifier or this shall not appear more than once in a + // lambda-capture. + if (CaptureMap.count(Var)) { + Diag(C->Loc, diag::err_capture_more_than_once) + << C->Id + << SourceRange(Captures[CaptureMap[Var]].getLocation()); + continue; + } // FIXME: If this is capture by copy, make sure that we can in fact copy // the variable. - Captures.push_back(LambdaScopeInfo::Capture(Var, C->Kind == LCK_ByRef, - /*isNested*/false, 0)); CaptureMap[Var] = Captures.size(); + Captures.push_back(LambdaScopeInfo::Capture(Var, C->Kind == LCK_ByRef, + /*isNested*/false, C->Loc, 0)); } // Build the call operator; we don't really have all the relevant information @@ -4951,6 +4980,9 @@ void Sema::ActOnStartOfLambdaDefinition(LambdaIntroducer &Intro, QualType MethodTy; TypeSourceInfo *MethodTyInfo; if (ParamInfo.getNumTypeObjects() == 0) { + // C++11 [expr.prim.lambda]p4: + // If a lambda-expression does not include a lambda-declarator, it is as + // if the lambda-declarator were (). FunctionProtoType::ExtProtoInfo EPI; EPI.TypeQuals |= DeclSpec::TQ_const; MethodTy = Context.getFunctionType(Context.DependentTy, @@ -4960,6 +4992,11 @@ void Sema::ActOnStartOfLambdaDefinition(LambdaIntroducer &Intro, assert(ParamInfo.isFunctionDeclarator() && "lambda-declarator is a function"); DeclaratorChunk::FunctionTypeInfo &FTI = ParamInfo.getFunctionTypeInfo(); + + // C++11 [expr.prim.lambda]p5: + // This function call operator is declared const (9.3.1) if and only if + // the lambda- expression’s parameter-declaration-clause is not followed + // by mutable. It is neither virtual nor declared volatile. if (!FTI.hasMutableQualifier()) FTI.TypeQuals |= DeclSpec::TQ_const; MethodTyInfo = GetTypeForDeclarator(ParamInfo, CurScope); @@ -4969,6 +5006,11 @@ void Sema::ActOnStartOfLambdaDefinition(LambdaIntroducer &Intro, assert(!MethodTy.isNull() && "no type from lambda declarator"); } + // C++11 [expr.prim.lambda]p5: + // The closure type for a lambda-expression has a public inline function + // call operator (13.5.4) whose parameters and return type are described by + // the lambda-expression’s parameter-declaration-clause and + // trailing-return-type respectively. DeclarationName MethodName = Context.DeclarationNames.getCXXOperatorName(OO_Call); CXXMethodDecl *Method diff --git a/lib/Sema/SemaExprObjC.cpp b/lib/Sema/SemaExprObjC.cpp index 4179ff870e..483e7eab23 100644 --- a/lib/Sema/SemaExprObjC.cpp +++ b/lib/Sema/SemaExprObjC.cpp @@ -265,7 +265,8 @@ ObjCMethodDecl *Sema::tryCaptureObjCSelf() { if (captureIndex) break; bool nested = isa(FunctionScopes[idx-1]); - blockScope->AddCapture(self, /*byref*/ false, nested, /*copy*/ 0); + blockScope->AddCapture(self, /*byref*/ false, nested, self->getLocation(), + /*copy*/ 0); captureIndex = blockScope->Captures.size(); // +1 } diff --git a/test/CXX/expr/expr.prim/expr.prim.lambda/p10.cpp b/test/CXX/expr/expr.prim/expr.prim.lambda/p10.cpp new file mode 100644 index 0000000000..497307ff75 --- /dev/null +++ b/test/CXX/expr/expr.prim/expr.prim.lambda/p10.cpp @@ -0,0 +1,24 @@ +// RUN: %clang_cc1 -std=c++11 %s -verify + +int GlobalVar; // expected-note 2{{declared here}} + +namespace N { + int AmbiguousVar; // expected-note {{candidate}} +} +int AmbiguousVar; // expected-note {{candidate}} +using namespace N; + +class X0 { + int Member; + + static void Overload(int); + void Overload(); + virtual X0& Overload(float); + + void explicit_capture() { + [&Overload] () {}; // expected-error {{does not name a variable}} expected-error {{not supported yet}} + [&GlobalVar] () {}; // expected-error {{does not have automatic storage duration}} expected-error {{not supported yet}} + [&AmbiguousVar] () {} // expected-error {{reference to 'AmbiguousVar' is ambiguous}} expected-error {{not supported yet}} + [&Globalvar] () {}; // expected-error {{use of undeclared identifier 'Globalvar'; did you mean 'GlobalVar}} + } +}; diff --git a/test/CXX/expr/expr.prim/expr.prim.lambda/p8.cpp b/test/CXX/expr/expr.prim/expr.prim.lambda/p8.cpp new file mode 100644 index 0000000000..4bd5760bc4 --- /dev/null +++ b/test/CXX/expr/expr.prim/expr.prim.lambda/p8.cpp @@ -0,0 +1,16 @@ +// RUN: %clang_cc1 -std=c++11 %s -verify + +class X0 { + void explicit_capture() { + int foo; + + [foo, foo] () {}; // expected-error {{'foo' can appear only once}} expected-error {{not supported yet}} + [this, this] () {}; // expected-error {{'this' can appear only once}} expected-error {{not supported yet}} + [=, foo] () {}; // expected-error {{'&' must precede a capture when}} expected-error {{not supported yet}} + [=, &foo] () {}; // expected-error {{not supported yet}} + [=, this] () {}; // expected-error {{'this' cannot appear}} expected-error {{not supported yet}} + [&, foo] () {}; // expected-error {{not supported yet}} + [&, &foo] () {}; // expected-error {{'&' cannot precede a capture when}} expected-error {{not supported yet}} + [&, this] () {}; // expected-error {{not supported yet}} + } +}; diff --git a/test/SemaCXX/lambda-expressions.cpp b/test/SemaCXX/lambda-expressions.cpp index 0223b50755..cacd4b680c 100644 --- a/test/SemaCXX/lambda-expressions.cpp +++ b/test/SemaCXX/lambda-expressions.cpp @@ -3,14 +3,6 @@ namespace std { class type_info; }; namespace ExplicitCapture { - int GlobalVar; // expected-note {{declared here}} - - namespace N { - int AmbiguousVar; // expected-note {{candidate}} - } - int AmbiguousVar; // expected-note {{candidate}} - using namespace N; - class C { int Member; @@ -18,28 +10,12 @@ namespace ExplicitCapture { void Overload(); virtual C& Overload(float); - void ExplicitCapture() { - int foo; - - [foo, foo] () {}; // expected-error {{'foo' can appear only once}} expected-error {{not supported yet}} - [this, this] () {}; // expected-error {{'this' can appear only once}} expected-error {{not supported yet}} - [=, foo] () {}; // expected-error {{'&' must precede a capture when}} expected-error {{not supported yet}} - [=, &foo] () {}; // expected-error {{not supported yet}} - [=, this] () {}; // expected-error {{'this' cannot appear}} expected-error {{not supported yet}} - [&, foo] () {}; // expected-error {{not supported yet}} - [&, &foo] () {}; // expected-error {{'&' cannot precede a capture when}} expected-error {{not supported yet}} - [&, this] () {}; // expected-error {{not supported yet}} - [&Overload] () {}; // expected-error {{does not name a variable}} expected-error {{not supported yet}} - [&GlobalVar] () {}; // expected-error {{does not have automatic storage duration}} expected-error {{not supported yet}} - [&AmbiguousVar] () {} // expected-error {{reference to 'AmbiguousVar' is ambiguous}} expected-error {{not supported yet}} - [&Globalvar] () {}; // expected-error {{use of undeclared identifier 'Globalvar'; did you mean 'GlobalVar}} - } - void ImplicitThisCapture() { [](){(void)Member;}; // expected-error {{'this' cannot be implicitly captured in this context}} expected-error {{not supported yet}} [&](){(void)Member;}; // expected-error {{not supported yet}} - [this](){(void)Member;}; // expected-error {{not supported yet}} - [this]{[this]{};};// expected-error 2 {{not supported yet}} + // FIXME: 'this' captures below don't actually work yet + // FIXME: [this](){(void)Member;}; + // FIXME: [this]{[this]{};}; []{[this]{};};// expected-error {{'this' cannot be implicitly captured in this context}} expected-error 2 {{not supported yet}} []{Overload(3);}; // expected-error {{not supported yet}} []{Overload();}; // expected-error {{'this' cannot be implicitly captured in this context}} expected-error {{not supported yet}}