Reverted in r369677. On Thu, 22 Aug 2019 at 10:34, Richard Smith <rich...@metafoo.co.uk> wrote:
> 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@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@lists.llvm.org >> https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits >> >
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits