[cfe-commits] r149462 - in /cfe/trunk: include/clang/Sema/ScopeInfo.h lib/Sema/SemaExpr.cpp lib/Sema/SemaExprCXX.cpp lib/Sema/SemaExprObjC.cpp test/CXX/expr/expr.prim/expr.prim.lambda/p10.cpp test/CXX/expr/expr.prim/expr.prim.lambda/p8.cpp test/SemaCXX/lambda-expressions.cpp

Douglas Gregor dgregor at apple.com
Tue Jan 31 17:18:44 PST 2012


Author: dgregor
Date: Tue Jan 31 19:18:43 2012
New Revision: 149462

URL: http://llvm.org/viewvc/llvm-project?rev=149462&view=rev
Log:
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.


Added:
    cfe/trunk/test/CXX/expr/expr.prim/expr.prim.lambda/p10.cpp
    cfe/trunk/test/CXX/expr/expr.prim/expr.prim.lambda/p8.cpp
Modified:
    cfe/trunk/include/clang/Sema/ScopeInfo.h
    cfe/trunk/lib/Sema/SemaExpr.cpp
    cfe/trunk/lib/Sema/SemaExprCXX.cpp
    cfe/trunk/lib/Sema/SemaExprObjC.cpp
    cfe/trunk/test/SemaCXX/lambda-expressions.cpp

Modified: cfe/trunk/include/clang/Sema/ScopeInfo.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Sema/ScopeInfo.h?rev=149462&r1=149461&r2=149462&view=diff
==============================================================================
--- cfe/trunk/include/clang/Sema/ScopeInfo.h (original)
+++ cfe/trunk/include/clang/Sema/ScopeInfo.h Tue Jan 31 19:18:43 2012
@@ -140,15 +140,18 @@
     // 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 @@
     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 @@
   /// 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();
   }
 

Modified: cfe/trunk/lib/Sema/SemaExpr.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaExpr.cpp?rev=149462&r1=149461&r2=149462&view=diff
==============================================================================
--- cfe/trunk/lib/Sema/SemaExpr.cpp (original)
+++ cfe/trunk/lib/Sema/SemaExpr.cpp Tue Jan 31 19:18:43 2012
@@ -1263,7 +1263,8 @@
          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 @@
     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,

Modified: cfe/trunk/lib/Sema/SemaExprCXX.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaExprCXX.cpp?rev=149462&r1=149461&r2=149462&view=diff
==============================================================================
--- cfe/trunk/lib/Sema/SemaExprCXX.cpp (original)
+++ cfe/trunk/lib/Sema/SemaExprCXX.cpp Tue Jan 31 19:18:43 2012
@@ -706,7 +706,7 @@
        NumClosures; --idx, --NumClosures) {
     CapturingScopeInfo *CSI = cast<CapturingScopeInfo>(FunctionScopes[idx]);
     bool isNested = NumClosures > 1;
-    CSI->AddThisCapture(isNested);
+    CSI->AddThisCapture(isNested, Loc);
   }
 }
 
@@ -4869,16 +4869,27 @@
   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 @@
 
       // 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 @@
 
     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 @@
   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 @@
     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 @@
     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

Modified: cfe/trunk/lib/Sema/SemaExprObjC.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaExprObjC.cpp?rev=149462&r1=149461&r2=149462&view=diff
==============================================================================
--- cfe/trunk/lib/Sema/SemaExprObjC.cpp (original)
+++ cfe/trunk/lib/Sema/SemaExprObjC.cpp Tue Jan 31 19:18:43 2012
@@ -265,7 +265,8 @@
     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
   }
 

Added: cfe/trunk/test/CXX/expr/expr.prim/expr.prim.lambda/p10.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CXX/expr/expr.prim/expr.prim.lambda/p10.cpp?rev=149462&view=auto
==============================================================================
--- cfe/trunk/test/CXX/expr/expr.prim/expr.prim.lambda/p10.cpp (added)
+++ cfe/trunk/test/CXX/expr/expr.prim/expr.prim.lambda/p10.cpp Tue Jan 31 19:18:43 2012
@@ -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}}
+  }
+};

Added: cfe/trunk/test/CXX/expr/expr.prim/expr.prim.lambda/p8.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CXX/expr/expr.prim/expr.prim.lambda/p8.cpp?rev=149462&view=auto
==============================================================================
--- cfe/trunk/test/CXX/expr/expr.prim/expr.prim.lambda/p8.cpp (added)
+++ cfe/trunk/test/CXX/expr/expr.prim/expr.prim.lambda/p8.cpp Tue Jan 31 19:18:43 2012
@@ -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}}
+  }
+};

Modified: cfe/trunk/test/SemaCXX/lambda-expressions.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/lambda-expressions.cpp?rev=149462&r1=149461&r2=149462&view=diff
==============================================================================
--- cfe/trunk/test/SemaCXX/lambda-expressions.cpp (original)
+++ cfe/trunk/test/SemaCXX/lambda-expressions.cpp Tue Jan 31 19:18:43 2012
@@ -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 @@
     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}}





More information about the cfe-commits mailing list