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

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@369591 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 2a72f7d5c61d9dac9ac82c3f4808c88a0353872d..b694c058a3a37b0d3486ec8f46ed533ce582eea4 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 289a1403cd84cde890cebc9e884331bb1f60da90..ee9963fca779b705a278aa5b29ead6317089f613 100644 (file)
--- a/lib/Sema/SemaInit.cpp
+++ b/lib/Sema/SemaInit.cpp
@@ -6561,7 +6561,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;
 }
 
@@ -6672,7 +6672,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 25d0db803983880945b07bf1ff6507e46edd7bff..2e76dc1a5843cb56100f4a4726d1c3f16c284173 100644 (file)
--- a/lib/Sema/SemaTemplateInstantiateDecl.cpp
+++ b/lib/Sema/SemaTemplateInstantiateDecl.cpp
@@ -552,6 +552,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
@@ -711,6 +723,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