Reland [LifetimeAnalysis] Support more STL idioms (template forward declaration and...

Reland after https://reviews.llvm.org/D66806 fixed the false-positive diagnostics.

Summary:
This fixes inference of gsl::Pointer on std::set::iterator with libstdc++ (the typedef for iterator
on the template is a DependentNameType - we can only put the gsl::Pointer attribute
on the underlaying record after instantiation)

inference of gsl::Pointer on std::vector::iterator with libc++ (the class was forward-declared,
we added the gsl::Pointer on the canonical decl (the forward decl), and later when the
template was instantiated, there was no attribute on the definition so it was not instantiated).

and a duplicate gsl::Pointer on some class with libstdc++ (we first added an attribute to
a incomplete instantiation, and then another was copied from the template definition
when the instantiation was completed).

We now add the attributes to all redeclarations to fix thos issues and make their usage easier.

Reviewers: gribozavr

Subscribers: Szelethus, xazax.hun, cfe-commits

Tags: #clang

Differential Revision: https://reviews.llvm.org/D66179

git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@371182 91177308-0d34-0410-b5e6-96231b3b80d8

diff --git a/lib/Sema/SemaAttr.cpp b/lib/Sema/SemaAttr.cpp
index e2542edf6ed5072228e6fccd6568d83871f10e39..23cb47481f3d219607a06e6dc2c7c10855b5d98c 100644 (file)
--- a/lib/Sema/SemaAttr.cpp
+++ b/lib/Sema/SemaAttr.cpp
@@ -88,13 +88,11 @@ void Sema::AddMsStructLayoutForRecord(RecordDecl *RD) {
 template <typename Attribute>
 static void addGslOwnerPointerAttributeIfNotExisting(ASTContext &Context,
                                                      CXXRecordDecl *Record) {
-  CXXRecordDecl *Canonical = Record->getCanonicalDecl();
-  if (Canonical->hasAttr<OwnerAttr>() || Canonical->hasAttr<PointerAttr>())
+  if (Record->hasAttr<OwnerAttr>() || Record->hasAttr<PointerAttr>())
     return;
 
-  Canonical->addAttr(::new (Context) Attribute(SourceRange{}, Context,
-                                               /*DerefType*/ nullptr,
-                                               /*Spelling=*/0));
+  for (Decl *Redecl : Record->redecls())
+    Redecl->addAttr(Attribute::CreateImplicit(Context, /*DerefType=*/nullptr));
 }
 
 void Sema::inferGslPointerAttribute(NamedDecl *ND,
@@ -189,8 +187,7 @@ void Sema::inferGslOwnerPointerAttribute(CXXRecordDecl *Record) {
 
   // Handle classes that directly appear in std namespace.
   if (Record->isInStdNamespace()) {
-    CXXRecordDecl *Canonical = Record->getCanonicalDecl();
-    if (Canonical->hasAttr<OwnerAttr>() || Canonical->hasAttr<PointerAttr>())
+    if (Record->hasAttr<OwnerAttr>() || Record->hasAttr<PointerAttr>())
       return;
 
     if (StdOwners.count(Record->getName()))

diff --git a/lib/Sema/SemaDeclAttr.cpp b/lib/Sema/SemaDeclAttr.cpp
index 2890f909dc490650ea51367aa0b1226fb04bfee9..79406707a4de0643a2d2ab40136dcb42e7b5bcb7 100644 (file)
--- a/lib/Sema/SemaDeclAttr.cpp
+++ b/lib/Sema/SemaDeclAttr.cpp
@@ -4592,9 +4592,11 @@ static void handleLifetimeCategoryAttr(Sema &S, Decl *D, const ParsedAttr &AL) {
       }
       return;
     }
-    D->addAttr(::new (S.Context)
-                   OwnerAttr(AL.getRange(), S.Context, DerefTypeLoc,
-                             AL.getAttributeSpellingListIndex()));
+    for (Decl *Redecl : D->redecls()) {
+      Redecl->addAttr(::new (S.Context)
+                          OwnerAttr(AL.getRange(), S.Context, DerefTypeLoc,
+                                    AL.getAttributeSpellingListIndex()));
+    }
   } else {
     if (checkAttrMutualExclusion<OwnerAttr>(S, D, AL))
       return;
@@ -4609,9 +4611,11 @@ static void handleLifetimeCategoryAttr(Sema &S, Decl *D, const ParsedAttr &AL) {
       }
       return;
     }
-    D->addAttr(::new (S.Context)
-                   PointerAttr(AL.getRange(), S.Context, DerefTypeLoc,
-                               AL.getAttributeSpellingListIndex()));
+    for (Decl *Redecl : D->redecls()) {
+      Redecl->addAttr(::new (S.Context)
+                          PointerAttr(AL.getRange(), S.Context, DerefTypeLoc,
+                                      AL.getAttributeSpellingListIndex()));
+    }
   }
 }
 

diff --git a/lib/Sema/SemaInit.cpp b/lib/Sema/SemaInit.cpp
index ff9b225207c169f42a8403a33739fcec0fc20019..e18ae74c2510a7420be942f7e815fd5bfbe952fa 100644 (file)
--- a/lib/Sema/SemaInit.cpp
+++ b/lib/Sema/SemaInit.cpp
@@ -6693,7 +6693,7 @@ static void visitLocalsRetainedByReferenceBinding(IndirectLocalPath &Path,
 
 template <typename T> static bool isRecordWithAttr(QualType Type) {
   if (auto *RD = Type->getAsCXXRecordDecl())
-    return RD->getCanonicalDecl()->hasAttr<T>();
+    return RD->hasAttr<T>();
   return false;
 }
 
@@ -6810,7 +6810,7 @@ static void handleGslAnnotatedTypes(IndirectLocalPath &Path, Expr *Call,
 
   if (auto *CCE = dyn_cast<CXXConstructExpr>(Call)) {
     const auto *Ctor = CCE->getConstructor();
-    const CXXRecordDecl *RD = Ctor->getParent()->getCanonicalDecl();
+    const CXXRecordDecl *RD = Ctor->getParent();
     if (CCE->getNumArgs() > 0 && RD->hasAttr<PointerAttr>())
       VisitPointerArg(Ctor->getParamDecl(0), CCE->getArgs()[0]);
   }

diff --git a/lib/Sema/SemaTemplateInstantiateDecl.cpp b/lib/Sema/SemaTemplateInstantiateDecl.cpp
index 5ef16b4a3751fe61e3dfc9d27d71ac0c13287422..908230bfe2f9a40e4472d81262dc0895f1722d78 100644 (file)
--- a/lib/Sema/SemaTemplateInstantiateDecl.cpp
+++ b/lib/Sema/SemaTemplateInstantiateDecl.cpp
@@ -550,6 +550,18 @@ void Sema::InstantiateAttrs(const MultiLevelTemplateArgumentList &TemplateArgs,
       continue;
     }
 
+    if (auto *A = dyn_cast<PointerAttr>(TmplAttr)) {
+      if (!New->hasAttr<PointerAttr>())
+        New->addAttr(A->clone(Context));
+      continue;
+    }
+
+    if (auto *A = dyn_cast<OwnerAttr>(TmplAttr)) {
+      if (!New->hasAttr<OwnerAttr>())
+        New->addAttr(A->clone(Context));
+      continue;
+    }
+
     assert(!TmplAttr->isPackExpansion());
     if (TmplAttr->isLateParsed() && LateAttrs) {
       // Late parsed attributes must be instantiated and attached after the
@@ -709,6 +721,9 @@ Decl *TemplateDeclInstantiator::InstantiateTypedefNameDecl(TypedefNameDecl *D,
 
   SemaRef.InstantiateAttrs(TemplateArgs, D, Typedef);
 
+  if (D->getUnderlyingType()->getAs<DependentNameType>())
+    SemaRef.inferGslPointerAttribute(Typedef);
+
   Typedef->setAccess(D->getAccess());
 
   return Typedef;

diff --git a/test/SemaCXX/attr-gsl-owner-pointer-std.cpp b/test/SemaCXX/attr-gsl-owner-pointer-std.cpp
index 29675c2ac7e9b072f14be8d665d8c88d98ba48c1..352e1e473580a682bfc265b389fd72aabbe0cd06 100644 (file)
--- a/test/SemaCXX/attr-gsl-owner-pointer-std.cpp
+++ b/test/SemaCXX/attr-gsl-owner-pointer-std.cpp
@@ -92,6 +92,59 @@ public:
 static_assert(sizeof(unordered_map<int>::iterator), ""); // Force instantiation.
 } // namespace inlinens
 
+// The iterator typedef is a DependentNameType.
+template <typename T>
+class __unordered_multimap_iterator {};
+// CHECK: ClassTemplateDecl {{.*}} __unordered_multimap_iterator
+// CHECK: ClassTemplateSpecializationDecl {{.*}} __unordered_multimap_iterator
+// CHECK: TemplateArgument type 'int'
+// CHECK: PointerAttr
+
+template <typename T>
+class __unordered_multimap_base {
+public:
+  using iterator = __unordered_multimap_iterator<T>;
+};
+
+template <typename T>
+class unordered_multimap {
+  // CHECK: ClassTemplateDecl {{.*}} unordered_multimap
+  // CHECK: OwnerAttr {{.*}}
+  // CHECK: ClassTemplateSpecializationDecl {{.*}} unordered_multimap
+  // CHECK: OwnerAttr {{.*}}
+public:
+  using _Mybase = __unordered_multimap_base<T>;
+  using iterator = typename _Mybase::iterator;
+};
+static_assert(sizeof(unordered_multimap<int>::iterator), ""); // Force instantiation.
+
+// The canonical declaration of the iterator template is not its definition.
+template <typename T>
+class __unordered_multiset_iterator;
+// CHECK: ClassTemplateDecl {{.*}} __unordered_multiset_iterator
+// CHECK: PointerAttr
+// CHECK: ClassTemplateSpecializationDecl {{.*}} __unordered_multiset_iterator
+// CHECK: TemplateArgument type 'int'
+// CHECK: PointerAttr
+
+template <typename T>
+class __unordered_multiset_iterator {
+  // CHECK: ClassTemplateDecl {{.*}} prev {{.*}} __unordered_multiset_iterator
+  // CHECK: PointerAttr
+};
+
+template <typename T>
+class unordered_multiset {
+  // CHECK: ClassTemplateDecl {{.*}} unordered_multiset
+  // CHECK: OwnerAttr {{.*}}
+  // CHECK: ClassTemplateSpecializationDecl {{.*}} unordered_multiset
+  // CHECK: OwnerAttr {{.*}}
+public:
+  using iterator = __unordered_multiset_iterator<T>;
+};
+
+static_assert(sizeof(unordered_multiset<int>::iterator), ""); // Force instantiation.
+
 // std::list has an implicit gsl::Owner attribute,
 // but explicit attributes take precedence.
 template <typename T>

diff --git a/test/SemaCXX/attr-gsl-owner-pointer.cpp b/test/SemaCXX/attr-gsl-owner-pointer.cpp
index 1c3deb3982e85ebae6c23cfaf3a049dc513ffac4..5b438822ba21b32c8eafae196740402b056bb2fb 100644 (file)
--- a/test/SemaCXX/attr-gsl-owner-pointer.cpp
+++ b/test/SemaCXX/attr-gsl-owner-pointer.cpp
@@ -105,3 +105,20 @@ class [[gsl::Owner(int)]] AddTheSameLater{};
 class [[gsl::Owner(int)]] AddTheSameLater;
 // CHECK: CXXRecordDecl {{.*}} prev {{.*}} AddTheSameLater
 // CHECK: OwnerAttr {{.*}} int
+
+template <class T>
+class [[gsl::Owner]] ForwardDeclared;
+// CHECK: ClassTemplateDecl {{.*}} ForwardDeclared
+// CHECK: OwnerAttr {{.*}}
+// CHECK: ClassTemplateSpecializationDecl {{.*}} ForwardDeclared
+// CHECK: TemplateArgument type 'int'
+// CHECK: OwnerAttr {{.*}}
+
+template <class T>
+class [[gsl::Owner]] ForwardDeclared {
+// CHECK: ClassTemplateDecl {{.*}} ForwardDeclared
+// CHECK: CXXRecordDecl {{.*}} ForwardDeclared definition
+// CHECK: OwnerAttr {{.*}}
+};
+
+static_assert(sizeof(ForwardDeclared<int>), ""); // Force instantiation.

diff --git a/unittests/Sema/CMakeLists.txt b/unittests/Sema/CMakeLists.txt
index 51e8d6c5b430c2cfba7c17e890ebacaac31f5f92..6832908e7fb1e05fc0e3431f6092270c264b2ee9 100644 (file)
--- a/unittests/Sema/CMakeLists.txt
+++ b/unittests/Sema/CMakeLists.txt
@@ -5,11 +5,13 @@ set(LLVM_LINK_COMPONENTS
 add_clang_unittest(SemaTests
   ExternalSemaSourceTest.cpp
   CodeCompleteTest.cpp
+  GslOwnerPointerInference.cpp
   )
 
 clang_target_link_libraries(SemaTests
   PRIVATE
   clangAST
+  clangASTMatchers
   clangBasic
   clangFrontend
   clangParse

diff --git a/unittests/Sema/GslOwnerPointerInference.cpp b/unittests/Sema/GslOwnerPointerInference.cpp
new file mode 100644 (file)
index 0000000..e340ccb
--- /dev/null
+++ b/unittests/Sema/GslOwnerPointerInference.cpp
@@ -0,0 +1,61 @@
+//== unittests/Sema/GslOwnerPointerInference.cpp - gsl::Owner/Pointer ========//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#include "../ASTMatchers/ASTMatchersTest.h"
+#include "clang/ASTMatchers/ASTMatchers.h"
+#include "gtest/gtest.h"
+
+namespace clang {
+using namespace ast_matchers;
+
+TEST(OwnerPointer, BothHaveAttributes) {
+  EXPECT_TRUE(matches(
+    R"cpp(
+      template<class T>
+      class [[gsl::Owner]] C;
+
+      template<class T>
+      class [[gsl::Owner]] C {};
+
+      C<int> c;
+  )cpp",
+    classTemplateSpecializationDecl(hasName("C"), hasAttr(clang::attr::Owner))));
+}
+
+TEST(OwnerPointer, ForwardDeclOnly) {
+  EXPECT_TRUE(matches(
+    R"cpp(
+      template<class T>
+      class [[gsl::Owner]] C;
+
+      template<class T>
+      class C {};
+
+      C<int> c;
+  )cpp",
+    classTemplateSpecializationDecl(hasName("C"), hasAttr(clang::attr::Owner))));
+}
+
+TEST(OwnerPointer, LateForwardDeclOnly) {
+  EXPECT_TRUE(matches(
+    R"cpp(
+      template<class T>
+      class C;
+
+      template<class T>
+      class C {};
+
+      template<class T>
+      class [[gsl::Owner]] C;
+
+      C<int> c;
+  )cpp",
+    classTemplateSpecializationDecl(hasName("C"), hasAttr(clang::attr::Owner))));
+}
+
+} // namespace clang