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:47:02 PDT 2019
Reverted in r369677.
On Thu, 22 Aug 2019 at 10:34, Richard Smith <richard at 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 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/8bcb48ee/attachment-0001.html>
More information about the cfe-commits
mailing list