Fix false negatives of statement local lifetime analysis for some STL implementation

[xazax.hun]

Summary

Some STL implementations have some class definitions (e.g. iterators) in a namespace outside of std.
This patch will extend the warnings from classes in std namespace to other implementation specified namespaces (that start with double underscored or underscore and a capital letter).

This patch also contains some NFC changes that supposed to make the code cleaner. If desired, I can split up this patch to two parts.

[xazax.hun]

clang/lib/Sema/SemaInit.cpp

7130

This is the NFC part.

[Szelethus]

Apologies for going off-topic, but would it make sense to start lifetime related patches with the tag [LifetimeAnalysis] or similar (like [analyzer] for static analyzer patches)? I dont feel knowledgable enough to participate, but would gladly add a herald rule to be subscribed and learn about the development process of it.

[xazax.hun]

In D66152#1627572, @Szelethus wrote:

Apologies for going off-topic, but would it make sense to start lifetime related patches with the tag [LifetimeAnalysis] or similar (like [analyzer] for static analyzer patches)? I dont feel knowledgable enough to participate, but would gladly add a herald rule to be subscribed and learn about the development process of it.

Makes sense :) I do not like to change titles as it will result in duplicate threads in the mail boxes but going forward I will definitely apply this.

[gribozavr]

clang/lib/Sema/SemaInit.cpp

6579

Please use isUppercase from include/clang/Basic/CharInfo.h.

clang/lib/Sema/SemaInit.cpp

 
 template<typename T>
 static bool isRecordWithAttr(QualType Type) {
   if (auto *RD = Type->getAsCXXRecordDecl())
     return RD->getCanonicalDecl()->hasAttr<T>();
   return false;
 }
 
+// Decl::isInStdNamespace will return false for iterators in some STL
+// implementations due to them being defined in a namespace outside of the std
+// namespace.
+static bool isInStlNamespace(const Decl *D) {
+  const DeclContext *DC = D->getDeclContext();
+  if (!DC)
+    return false;
+  if (const auto *ND = dyn_cast<NamespaceDecl>(DC))
+    if (const IdentifierInfo *II = ND->getIdentifier()) {
+      StringRef Name = II->getName();
+      if (Name.size() >= 2 && Name.front() == '_' &&
+          (Name[1] == '_' || llvm::toUpper(Name[1]) == Name[1]))
+        return true;
+    }
+
+  return DC->isStdNamespace();
+}
+
 static bool shouldTrackImplicitObjectArg(const CXXMethodDecl *Callee) {
   if (auto *Conv = dyn_cast_or_null<CXXConversionDecl>(Callee))
     if (isRecordWithAttr<PointerAttr>(Conv->getConversionType()))
       return true;
-  if (!Callee->getParent()->isInStdNamespace())
+  if (!isInStlNamespace(Callee->getParent()))
     return false;
   if (!isRecordWithAttr<PointerAttr>(Callee->getThisObjectType()) &&
       !isRecordWithAttr<OwnerAttr>(Callee->getThisObjectType()))
     return false;
   if (Callee->getReturnType()->isPointerType() ||
       isRecordWithAttr<PointerAttr>(Callee->getReturnType())) {
     if (!Callee->getIdentifier())
       return false;
if (LK == LK_FullExpression)
     return;
 
   auto TemporaryVisitor = [&](IndirectLocalPath &Path, Local L,
                               ReferenceKind RK) -> bool {
     SourceRange DiagRange = nextPathEntryRange(Path, 0, L);
     SourceLocation DiagLoc = DiagRange.getBegin();
 
     auto *MTE = dyn_cast<MaterializeTemporaryExpr>(L);
-    bool IsTempGslOwner = MTE && !MTE->getExtendingDecl() &&
-                          isRecordWithAttr<OwnerAttr>(MTE->getType());
-    bool IsLocalGslOwner =
-        isa<DeclRefExpr>(L) && isRecordWithAttr<OwnerAttr>(L->getType());
 
-    // Skipping a chain of initializing gsl::Pointer annotated objects.
-    // We are looking only for the final source to find out if it was
-    // a local or temporary owner or the address of a local variable/param. We
-    // do not want to follow the references when returning a pointer originating
+    bool IsGslPtrInitWithGslTempOwner = false;
+    bool IsLocalGslOwner = false;
+    if (pathOnlyInitializesGslPointer(Path)) {
+      if (isa<DeclRefExpr>(L)) {
+        // We do not want to follow the references when returning a pointer originating
         // from a local owner to avoid the following false positive:
         //   int &p = *localUniquePtr;
         //   someContainer.add(std::move(localUniquePtr));
         //   return p;
-    if (!IsTempGslOwner && pathOnlyInitializesGslPointer(Path) &&
-        !(IsLocalGslOwner && !pathContainsInit(Path)))
+        IsLocalGslOwner = isRecordWithAttr<OwnerAttr>(L->getType());
+        if (pathContainsInit(Path) || !IsLocalGslOwner)
+          return false;
+      } else {
+        IsGslPtrInitWithGslTempOwner = MTE && !MTE->getExtendingDecl() &&
+                            isRecordWithAttr<OwnerAttr>(MTE->getType());
+        // Skipping a chain of initializing gsl::Pointer annotated objects.
+        // We are looking only for the final source to find out if it was
+        // a local or temporary owner or the address of a local variable/param.
+        if (!IsGslPtrInitWithGslTempOwner)
           return true;
-
-    bool IsGslPtrInitWithGslTempOwner =
-        IsTempGslOwner && pathOnlyInitializesGslPointer(Path);
+      }
+    }
 
     switch (LK) {
     case LK_FullExpression:
       llvm_unreachable("already handled this");
 
     case LK_Extended: {
       if (!MTE) {
         // The initialized entity has lifetime beyond the full-expression,

clang/test/Sema/warn-lifetime-analysis-nocfg.cpp

void initLocalGslPtrWithTempOwner() {
   MyIntPointer p = MyIntOwner{}; // expected-warning {{object backing the pointer will be destroyed at the end of the full-expression}}
   p = MyIntOwner{}; // TODO ?
   global = MyIntOwner{}; // TODO ?
   MyLongPointerFromConversion p2 = MyLongOwnerWithConversion{}; // expected-warning {{object backing the pointer will be destroyed at the end of the full-expression}}
   p2 = MyLongOwnerWithConversion{}; // TODO ?
   global2 = MyLongOwnerWithConversion{}; // TODO ?
 }
 
-namespace std {
-template<class T> struct remove_reference       { typedef T type; };
-template<class T> struct remove_reference<T &>  { typedef T type; };
-template<class T> struct remove_reference<T &&> { typedef T type; };
-
-template<class T>
-typename remove_reference<T>::type &&move(T &&t) noexcept;
-
+namespace __gnu_cxx {
 template <typename T>
 struct basic_iterator {
   basic_iterator operator++();
   T& operator*() const;
 };
 
 template<typename T>
 bool operator!=(basic_iterator<T>, basic_iterator<T>);
+}
+
+namespace std {
+template<class T> struct remove_reference       { typedef T type; };
+template<class T> struct remove_reference<T &>  { typedef T type; };
+template<class T> struct remove_reference<T &&> { typedef T type; };
+
+template<class T>
+typename remove_reference<T>::type &&move(T &&t) noexcept;
 
 template <typename T>
 struct vector {
-  typedef basic_iterator<T> iterator;
+  typedef __gnu_cxx::basic_iterator<T> iterator;
   iterator begin();
   iterator end();
   const T *data() const;
   T &at(int n);
 };
 
 template<typename T>
 struct basic_string_view {