address comments

Author: usx95

a-84635e71.o.tmp

@@ -3922,8 +3922,8 @@ def LifetimeCaptureByDocs : Documentation {
   let Category = DocCatFunction;
   let Content = [{
     Similar to `lifetimebound`_, the ``lifetime_capture_by(X)`` attribute on a function
-parameter or implicit object parameter indicates that that objects that are referred to
-by that parameter may also be referred to by the capturing entity ``X``.
+parameter or implicit object parameter indicates that objects that are referred to by
+that parameter may also be referred to by the capturing entity ``X``.
 
 By default:
 
@@ -3932,7 +3932,7 @@ By default:
 - A ``std::initializer_list<T>`` is considered to refer to its underlying array.
 - Aggregates (arrays and simple ``struct``\s) are considered to refer to all
   objects that their transitive subobjects refer to.
-- View types (types annotated with [[``gsl::Pointer()``]]) are considered to refer
+- View types (types annotated with ``[[gsl::Pointer()]]``) are considered to refer
   to its pointee. This holds true even if the view type appears as a reference.
   For example, both ``std::string_view`` and ``const std::string_view &`` are
   considered to refer to a ``std::string``.
@@ -3990,7 +3990,7 @@ The attribute supports specifying more than one capturing entities:
     s2.insert(a);
   }
 
-Currently clang would diagnose when a temporary is used as an argument to a
+Currently Clang would diagnose when a temporary is used as an argument to a
 parameter whose reference is captured.
 
 .. code-block:: c++

a-9a215dcd.o.tmp

@@ -254,7 +254,7 @@ static void visitLocalsRetainedByReferenceBinding(IndirectLocalPath &Path,
                                                   LocalVisitor Visit);
 
 template <typename T> static bool isRecordWithAttr(QualType Type) {
-  if (auto *RD = Type.getNonReferenceType()->getAsCXXRecordDecl())
+  if (auto *RD = Type->getAsCXXRecordDecl())
     return RD->hasAttr<T>();
   return false;
 }
@@ -1437,6 +1437,7 @@ checkExprLifetimeImpl(Sema &SemaRef, const InitializedEntity *InitEntity,
     // warnings or errors on inner temporaries within this one's initializer.
     return false;
   };
+
   bool HasReferenceBinding = Init->isGLValue();
   llvm::SmallVector<IndirectLocalPathEntry, 8> Path;
   switch (LK) {

a-d85fccc7.o.tmp

@@ -3233,46 +3233,41 @@ void Sema::CheckArgAlignment(SourceLocation Loc, NamedDecl *FDecl,
 void Sema::checkLifetimeCaptureBy(FunctionDecl *FD, bool IsMemberFunction,
                                   const Expr *ThisArg,
                                   ArrayRef<const Expr *> Args) {
-  auto GetArgAt = [&](int Idx) -> Expr * {
+  if (!FD || Args.empty())
+    return;
+  auto GetArgAt = [&](int Idx) -> const Expr * {
     if (Idx == LifetimeCaptureByAttr::GLOBAL ||
         Idx == LifetimeCaptureByAttr::UNKNOWN)
       return nullptr;
     if (IsMemberFunction && Idx == 0)
-      return const_cast<Expr *>(ThisArg);
-    return const_cast<Expr *>(Args[Idx - int(IsMemberFunction)]);
+      return ThisArg;
+    return Args[Idx - IsMemberFunction];
   };
-  for (unsigned I = 0; I < FD->getNumParams(); ++I) {
-    auto *CapturedByAttr =
-        FD->getParamDecl(I)->getAttr<LifetimeCaptureByAttr>();
-    if (!CapturedByAttr)
-      continue;
-    for (int CapturingParamIdx : CapturedByAttr->params()) {
-      Expr *Capturing = GetArgAt(CapturingParamIdx);
-      Expr *Captured = GetArgAt(I + IsMemberFunction);
+  auto HandleCaptureByAttr = [&](const LifetimeCaptureByAttr *Attr,
+                                 unsigned ArgIdx) {
+    if (!Attr)
+      return;
+    Expr *Captured = const_cast<Expr *>(GetArgAt(ArgIdx));
+    for (int CapturingParamIdx : Attr->params()) {
+      Expr *Capturing = const_cast<Expr *>(GetArgAt(CapturingParamIdx));
       CapturingEntity CE{Capturing};
       // Ensure that 'Captured' outlives the 'Capturing' entity.
       checkExprLifetime(*this, CE, Captured);
     }
-  }
-  // Check when the 'this' object is captured.
+  };
+  for (unsigned I = 0; I < FD->getNumParams(); ++I)
+    HandleCaptureByAttr(FD->getParamDecl(I)->getAttr<LifetimeCaptureByAttr>(),
+                        I + IsMemberFunction);
+  // Check when the implicit object param is captured.
   if (IsMemberFunction) {
     TypeSourceInfo *TSI = FD->getTypeSourceInfo();
     if (!TSI)
       return;
     AttributedTypeLoc ATL;
     for (TypeLoc TL = TSI->getTypeLoc();
          (ATL = TL.getAsAdjusted<AttributedTypeLoc>());
-         TL = ATL.getModifiedLoc()) {
-      auto *CapturedByAttr = ATL.getAttrAs<LifetimeCaptureByAttr>();
-      if (!CapturedByAttr)
-        continue;
-      Expr *Captured = GetArgAt(0);
-      for (int CapturingParamIdx : CapturedByAttr->params()) {
-        Expr *Capturing = GetArgAt(CapturingParamIdx);
-        CapturingEntity CE{Capturing};
-        checkExprLifetime(*this, CE, Captured);
-      }
-    }
+         TL = ATL.getModifiedLoc())
+      HandleCaptureByAttr(ATL.getAttrAs<LifetimeCaptureByAttr>(), 0);
   }
 }
 

a-f0f01220.o.tmp

@@ -17,11 +17,11 @@ void noCaptureInt(int i [[clang::lifetime_capture_by(x)]], X &x);
 std::string_view substr(const std::string& s [[clang::lifetimebound]]);
 std::string_view strcopy(const std::string& s);
 
-void captureSV(std::string_view s [[clang::lifetime_capture_by(x)]], X &x);
-void captureRValSV(std::string_view &&sv [[clang::lifetime_capture_by(x)]], X &x);
-void noCaptureSV(std::string_view sv, X &x);
-void captureS(const std::string &s [[clang::lifetime_capture_by(x)]], X &x);
-void captureRValS(std::string &&s [[clang::lifetime_capture_by(x)]], X &x);
+void captureStringView(std::string_view s [[clang::lifetime_capture_by(x)]], X &x);
+void captureRValStringView(std::string_view &&sv [[clang::lifetime_capture_by(x)]], X &x);
+void noCaptureStringView(std::string_view sv, X &x);
+void captureString(const std::string &s [[clang::lifetime_capture_by(x)]], X &x);
+void captureRValString(std::string &&s [[clang::lifetime_capture_by(x)]], X &x);
 
 const std::string& getLB(const std::string &s [[clang::lifetimebound]]);
 const std::string& getLB(std::string_view sv [[clang::lifetimebound]]);
@@ -38,8 +38,8 @@ void captureByGlobal(std::string_view s [[clang::lifetime_capture_by(global)]]);
 void captureByUnknown(std::string_view s [[clang::lifetime_capture_by(unknown)]]);
 
 void use() {
-  std::string_view local_sv;
-  std::string local_s;
+  std::string_view local_string_view;
+  std::string local_string;
   X x;
   // Capture an 'int'.
   int local;
@@ -51,34 +51,34 @@ void use() {
   noCaptureInt(local, x);
 
   // Capture using std::string_view.
-  captureSV(local_sv, x);
-  captureSV(std::string(), // expected-warning {{object whose reference is captured by 'x'}}
+  captureStringView(local_string_view, x);
+  captureStringView(std::string(), // expected-warning {{object whose reference is captured by 'x'}}
             x);
-  captureSV(substr(
+  captureStringView(substr(
       std::string() // expected-warning {{object whose reference is captured by 'x'}}
       ), x);
-  captureSV(substr(local_s), x);
-  captureSV(strcopy(std::string()), x);
-  captureRValSV(std::move(local_sv), x);
-  captureRValSV(std::string(), x); // expected-warning {{object whose reference is captured by 'x'}}
-  captureRValSV(std::string_view{"abcd"}, x);
-  captureRValSV(substr(local_s), x);
-  captureRValSV(substr(std::string()), x); // expected-warning {{object whose reference is captured by 'x'}}
-  captureRValSV(strcopy(std::string()), x);
-  noCaptureSV(local_sv, x);
-  noCaptureSV(std::string(), x);
-  noCaptureSV(substr(std::string()), x);
+  captureStringView(substr(local_string), x);
+  captureStringView(strcopy(std::string()), x);
+  captureRValStringView(std::move(local_string_view), x);
+  captureRValStringView(std::string(), x); // expected-warning {{object whose reference is captured by 'x'}}
+  captureRValStringView(std::string_view{"abcd"}, x);
+  captureRValStringView(substr(local_string), x);
+  captureRValStringView(substr(std::string()), x); // expected-warning {{object whose reference is captured by 'x'}}
+  captureRValStringView(strcopy(std::string()), x);
+  noCaptureStringView(local_string_view, x);
+  noCaptureStringView(std::string(), x);
+  noCaptureStringView(substr(std::string()), x);
 
   // Capture using std::string.
-  captureS(std::string(), x); // expected-warning {{object whose reference is captured by 'x'}}
-  captureS(local_s, x);
-  captureRValS(std::move(local_s), x);
-  captureRValS(std::string(), x); // expected-warning {{object whose reference is captured by 'x'}}
+  captureString(std::string(), x); // expected-warning {{object whose reference is captured by 'x'}}
+  captureString(local_string, x);
+  captureRValString(std::move(local_string), x);
+  captureRValString(std::string(), x); // expected-warning {{object whose reference is captured by 'x'}}
 
   // Capture with lifetimebound.
-  captureSV(getLB(std::string()), x); // expected-warning {{object whose reference is captured by 'x'}}
-  captureSV(getLB(substr(std::string())), x); // expected-warning {{object whose reference is captured by 'x'}}
-  captureSV(getLB(getLB(
+  captureStringView(getLB(std::string()), x); // expected-warning {{object whose reference is captured by 'x'}}
+  captureStringView(getLB(substr(std::string())), x); // expected-warning {{object whose reference is captured by 'x'}}
+  captureStringView(getLB(getLB(
     std::string()  // expected-warning {{object whose reference is captured by 'x'}}
     )), x);
   capturePointer(getPointerLB(std::string()), x); // expected-warning {{object whose reference is captured by 'x'}}
@@ -101,14 +101,23 @@ void use() {
   // capture by global.
   captureByGlobal(std::string()); // expected-warning {{object whose reference is captured will be destroyed at the end of the full-expression}}
   captureByGlobal(substr(std::string())); // expected-warning {{captured}}
-  captureByGlobal(local_s);
-  captureByGlobal(local_sv);
+  captureByGlobal(local_string);
+  captureByGlobal(local_string_view);
 
   // // capture by unknown.
   captureByGlobal(std::string()); // expected-warning {{object whose reference is captured will be destroyed at the end of the full-expression}}
   captureByGlobal(substr(std::string())); // expected-warning {{captured}}
-  captureByGlobal(local_s);
-  captureByGlobal(local_sv);
+  captureByGlobal(local_string);
+  captureByGlobal(local_string_view);
+}
+
+void captureDefaultArg(X &x, std::string_view s [[clang::lifetime_capture_by(x)]] = std::string());
+void useCaptureDefaultArg() {
+  X x;
+  captureDefaultArg(x); // FIXME: Diagnose temporary default arg.
+  captureDefaultArg(x, std::string("temp")); // expected-warning {{captured}}
+  std::string local;
+  captureDefaultArg(x, local);
 }
 
 template<typename T> struct IsPointerLikeTypeImpl : std::false_type {};
@@ -189,13 +198,16 @@ void container_of_pointers() {
   std::optional<std::string_view> optionalSV;
   vsv.push_back(optionalSV.value());
   vsv.push_back(getOptionalS().value()); // expected-warning {{object whose reference is captured by 'vsv'}}
-  vsv.push_back(getOptionalSV().value());
-  vsv.push_back(getOptionalMySV().value());
+  
+  // FIXME: Following 2 cases are false positives:
+  vsv.push_back(getOptionalSV().value()); // expected-warning {{object whose reference}}
+  vsv.push_back(getOptionalMySV().value());  // expected-warning {{object whose reference}}
 
   // (maybe) FIXME: We may choose to diagnose the following case.
   // This happens because 'MyStringViewNotPointer' is not marked as a [[gsl::Pointer]] but is derived from one.
   vsv.push_back(getOptionalMySVNotP().value()); // expected-warning {{object whose reference is captured by 'vsv'}}
 }
+
 namespace temporary_views {
 void capture1(std::string_view s [[clang::lifetime_capture_by(x)]], std::vector<std::string_view>& x);
 
@@ -217,4 +229,4 @@ void test1() {
   capture3(std::string_view(), x3);
   capture3(std::string(), x3); // expected-warning {{captured by 'x3'}}
 }
-}
+}