r369591 - [LifetimeAnalysis] Support more STL idioms (template forward declaration and DependentNameType)
Matthias Gehre via cfe-commits
cfe-commits at lists.llvm.org
Thu Aug 22 13:05:32 PDT 2019
Hi Diana,
hi Richard,
thank you for the feedback!
Diana,
I remember that some gcc version had issues with raw string literals inside
macros. I'll fix that to use normal
string literals instead.
Richard,
We are definitely want the gsl::Pointer-based warnings that are enabled by
default to be free of any false-positives.
As you expected, this is showing a problem we have in another part of our
analysis, and we will fix it before landing
this PR again.
Both, sorry for the breakage!
Am Do., 22. Aug. 2019 um 19:47 Uhr schrieb Richard Smith <
richard at metafoo.co.uk>:
> 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/ed284e2d/attachment-0001.html>
More information about the cfe-commits
mailing list