On Thu, Aug 22, 2019 at 4:05 PM Matthias Gehre via cfe-commits < cfe-commits@lists.llvm.org> wrote:
> 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. > I think it's only a problem if the raw string is in a macro. Instead of FOO(R"(asdf)"); you can do const char kStr[] = R"(asdf)"; FOO(kStr); and gcc should be happy. (In case raw strings buy you something over normal string literals.) > > 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 < > rich...@metafoo.co.uk>: > >> 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 >
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits