r369591 - [LifetimeAnalysis] Support more STL idioms (template forward declaration and DependentNameType)
Richard Smith via cfe-commits
cfe-commits at lists.llvm.org
Thu Aug 22 10:34:33 PDT 2019
Hi Matthias,
This introduces false positives into -Wreturn-stack-address for an example
such as:
#include <vector>
std::vector<int>::iterator downcast_to(std::vector<int>::iterator value) {
return *&value;
}
This breaks an internal build bot for us, so I'm going to revert this for
now (though I expect this isn't the cause of the problem, but is rather
exposing a problem elsewhere).
If we can make the gsl::Pointer diagnostics false-positive-free, that's
great, but otherwise we should use a different warning flag for the
warnings that involve these annotations and use -Wreturn-stack-address for
only the zero-false-positive cases that it historically diagnosed.
Thanks, and sorry for the trouble.
On Wed, 21 Aug 2019 at 15:07, Matthias Gehre via cfe-commits <
cfe-commits at lists.llvm.org> wrote:
> Author: mgehre
> Date: Wed Aug 21 15:08:59 2019
> New Revision: 369591
>
> URL: http://llvm.org/viewvc/llvm-project?rev=369591&view=rev
> Log:
> [LifetimeAnalysis] Support more STL idioms (template forward declaration
> and DependentNameType)
>
> 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
>
> Added:
> cfe/trunk/unittests/Sema/GslOwnerPointerInference.cpp
> Modified:
> cfe/trunk/lib/Sema/SemaAttr.cpp
> cfe/trunk/lib/Sema/SemaDeclAttr.cpp
> cfe/trunk/lib/Sema/SemaInit.cpp
> cfe/trunk/lib/Sema/SemaTemplateInstantiateDecl.cpp
> cfe/trunk/test/SemaCXX/attr-gsl-owner-pointer-std.cpp
> cfe/trunk/test/SemaCXX/attr-gsl-owner-pointer.cpp
> cfe/trunk/unittests/Sema/CMakeLists.txt
>
> Modified: cfe/trunk/lib/Sema/SemaAttr.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaAttr.cpp?rev=369591&r1=369590&r2=369591&view=diff
>
> ==============================================================================
> --- cfe/trunk/lib/Sema/SemaAttr.cpp (original)
> +++ cfe/trunk/lib/Sema/SemaAttr.cpp Wed Aug 21 15:08:59 2019
> @@ -88,13 +88,11 @@ void Sema::AddMsStructLayoutForRecord(Re
> 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
>
> // 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()))
>
> Modified: cfe/trunk/lib/Sema/SemaDeclAttr.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDeclAttr.cpp?rev=369591&r1=369590&r2=369591&view=diff
>
> ==============================================================================
> --- cfe/trunk/lib/Sema/SemaDeclAttr.cpp (original)
> +++ cfe/trunk/lib/Sema/SemaDeclAttr.cpp Wed Aug 21 15:08:59 2019
> @@ -4592,9 +4592,11 @@ static void handleLifetimeCategoryAttr(S
> }
> 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(S
> }
> 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()));
> + }
> }
> }
>
>
> Modified: cfe/trunk/lib/Sema/SemaInit.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaInit.cpp?rev=369591&r1=369590&r2=369591&view=diff
>
> ==============================================================================
> --- cfe/trunk/lib/Sema/SemaInit.cpp (original)
> +++ cfe/trunk/lib/Sema/SemaInit.cpp Wed Aug 21 15:08:59 2019
> @@ -6561,7 +6561,7 @@ static void visitLocalsRetainedByReferen
>
> 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(Indi
>
> 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]);
> }
>
> Modified: cfe/trunk/lib/Sema/SemaTemplateInstantiateDecl.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaTemplateInstantiateDecl.cpp?rev=369591&r1=369590&r2=369591&view=diff
>
> ==============================================================================
> --- cfe/trunk/lib/Sema/SemaTemplateInstantiateDecl.cpp (original)
> +++ cfe/trunk/lib/Sema/SemaTemplateInstantiateDecl.cpp Wed Aug 21 15:08:59
> 2019
> @@ -552,6 +552,18 @@ void Sema::InstantiateAttrs(const MultiL
> 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::Instanti
>
> SemaRef.InstantiateAttrs(TemplateArgs, D, Typedef);
>
> + if (D->getUnderlyingType()->getAs<DependentNameType>())
> + SemaRef.inferGslPointerAttribute(Typedef);
> +
> Typedef->setAccess(D->getAccess());
>
> return Typedef;
>
> Modified: cfe/trunk/test/SemaCXX/attr-gsl-owner-pointer-std.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/attr-gsl-owner-pointer-std.cpp?rev=369591&r1=369590&r2=369591&view=diff
>
> ==============================================================================
> --- cfe/trunk/test/SemaCXX/attr-gsl-owner-pointer-std.cpp (original)
> +++ cfe/trunk/test/SemaCXX/attr-gsl-owner-pointer-std.cpp Wed Aug 21
> 15:08:59 2019
> @@ -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>
>
> Modified: cfe/trunk/test/SemaCXX/attr-gsl-owner-pointer.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/attr-gsl-owner-pointer.cpp?rev=369591&r1=369590&r2=369591&view=diff
>
> ==============================================================================
> --- cfe/trunk/test/SemaCXX/attr-gsl-owner-pointer.cpp (original)
> +++ cfe/trunk/test/SemaCXX/attr-gsl-owner-pointer.cpp Wed Aug 21 15:08:59
> 2019
> @@ -105,3 +105,20 @@ class [[gsl::Owner(int)]] AddTheSameLate
> 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.
>
> Modified: cfe/trunk/unittests/Sema/CMakeLists.txt
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Sema/CMakeLists.txt?rev=369591&r1=369590&r2=369591&view=diff
>
> ==============================================================================
> --- cfe/trunk/unittests/Sema/CMakeLists.txt (original)
> +++ cfe/trunk/unittests/Sema/CMakeLists.txt Wed Aug 21 15:08:59 2019
> @@ -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
>
> Added: cfe/trunk/unittests/Sema/GslOwnerPointerInference.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Sema/GslOwnerPointerInference.cpp?rev=369591&view=auto
>
> ==============================================================================
> --- cfe/trunk/unittests/Sema/GslOwnerPointerInference.cpp (added)
> +++ cfe/trunk/unittests/Sema/GslOwnerPointerInference.cpp Wed Aug 21
> 15:08:59 2019
> @@ -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
>
>
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20190822/8df1ec57/attachment-0001.html>
More information about the cfe-commits
mailing list