================
@@ -347,6 +361,30 @@ static bool shouldTrackFirstArgument(const FunctionDecl 
*FD) {
   return false;
 }
 
+// Returns true if we should perform the GSL analysis on the first argument for
+// the given constructor.
+static bool
+shouldTrackFirstArgumentForConstructor(const CXXConstructExpr *Ctor) {
+  const auto *ClassD = Ctor->getConstructor()->getParent();
+
+  auto FirstArgType = Ctor->getArg(0)->getType();
+  // Case 1, construct a GSL pointer, e.g. std::string_view
+  if (ClassD->hasAttr<PointerAttr>())
+    return true;
+
+  // case 2: construct a container of pointer (std::vector<std::string_view>)
+  // from an owner or a std::initilizer_list.
+  //
+  // std::initializer_list is a proxy object that provides access to the 
backing
+  // array. We perform analysis on it to determine if there are any dangling
+  // temporaries in the backing array.
+  if (Ctor->getConstructor()->getNumParams() != 1 ||
+      !isContainerOfPointer(ClassD))
+    return false;
+  return isGSLOwner(FirstArgType) ||
+         isStdInitializerListOfPointer(FirstArgType->getAsRecordDecl());
----------------
hokein wrote:

> It looks like we mean to say that std::initialiser_list<Pointer> should be 
> considered as GSL Owner.

I think what we actually intend is to continue tracking the argument of type 
`std::initializer_list<Pointer>`. (while previously, we only tracked arguments 
for GSL Owner types.) Conceptually, `std::initializer_list<Pointer>` is not an 
owner, so moving this logic to `isGSLOwner` doesn't feel appropriate, even 
though it works in practice.

Regarding the new false negative, it was introduced when we replaced instances 
of isRecordWithAttr<OwnerAttr> in 
[TemporaryVisitor](https://github.com/llvm/llvm-project/blob/main/clang/lib/Sema/CheckExprLifetime.cpp#L988)
 with the new isGSLOwner function (which excludes `Owner<Pointer>` types). The 
logic in `TemporaryVisitor` is a bit subtle, which is to skip reporting issues 
if the local variable is not an owner type. Since `isGSLOwner` excludes 
`vector<int*>`, it no longer triggers a report.

To fix this, I reverted the abstraction of isGSLOwner because the 
`!isContainerOfPointer` logic only applies to determining whether to trace the 
first argument of the `container<pointer>` constructor. I've added comments 
there).


https://github.com/llvm/llvm-project/pull/108344
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to