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 7ffc3bcfb935a8a33b302750646e0a171605f074..bff2cf8ad22778fc49c17352a150007f1a320f75 100644 (file)
--- a/include/clang/Sema/ScopeInfo.h
+++ b/include/clang/Sema/ScopeInfo.h
@@ -140,15 +140,18 @@ public:
     // copy constructor.
     llvm::PointerIntPair<Expr*, 1, bool> 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 0d929f1836fb7108a4092f46c7a1b4c5f8164d36..5de0388c067ce147787dbf578752ba2f0c714b1f 100644 (file)
--- 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<BlockScopeInfo>(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<BlockScopeInfo>(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 a24063f86080f53ba72d8def2923e786835f97e8..0a987511fb69257ca8320f6df71f5b2a07ae265e 100644 (file)
--- 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<CapturingScopeInfo>(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<LambdaCapture, 4>::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<VarDecl> 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<VarDecl>();
     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<BlocksAttr>()) {
       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 4179ff870e8c4406ff1b3a3102c512c89da90012..483e7eab23230b6492364bc1da6d0567e970894e 100644 (file)
--- a/lib/Sema/SemaExprObjC.cpp
+++ b/lib/Sema/SemaExprObjC.cpp
@@ -265,7 +265,8 @@ ObjCMethodDecl *Sema::tryCaptureObjCSelf() {
     if (captureIndex) break;
 
     bool nested = isa<BlockScopeInfo>(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 (file)
index 0000000..497307f
--- /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 (file)
index 0000000..4bd5760
--- /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 0223b50755eed71323ccd26960860cf1e852cde8..cacd4b680ce9274311d89a6e18c2fd0418cdab2e 100644 (file)
--- 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}}