r300650 - [modules] Properly look up the owning module for an instantiation of a merged template.

Richard Smith via cfe-commits cfe-commits at lists.llvm.org
Tue Apr 18 23:11:07 PDT 2017


Thanks for the revert, I'll investigate tomorrow.

On 18 Apr 2017 10:41 pm, "Chandler Carruth" <chandlerc at google.com> wrote:

> Reverted in r300658.
>
> On Tue, Apr 18, 2017 at 8:30 PM Chandler Carruth <chandlerc at google.com>
> wrote:
>
>> Consider code like the following:
>>
>> ```
>> #include <memory>
>>
>> template <class T>
>> class MyAllocator : public std::allocator<T> {
>>  public:
>>   typedef std::allocator<T> Alloc;
>>   typedef typename Alloc::pointer pointer;
>>   typedef typename Alloc::size_type size_type;
>>
>>   MyAllocator() {}
>>
>>   template <typename U>
>>   MyAllocator(const MyAllocator<U>& x) : Alloc(x) {}
>>
>>   pointer allocate(size_type n,
>>                    std::allocator<void>::const_pointer /*hint*/ =
>> nullptr) {
>>     void* p = malloc(n * sizeof(T));
>>     return static_cast<pointer>(p);
>>   }
>>
>>   void deallocate(pointer p, size_type) { free(p); }
>>
>>   template <typename U>
>>   struct rebind {
>>     typedef MyAllocator<U> other;
>>   };
>>
>>  private:
>>   template <class U>
>>   friend class MyAllocator;
>> };
>>
>> std::shared_ptr<int> x = std::allocate_shared<int>(MyAllocator<int>(),
>> 0);
>> ```
>>
>> This will fail to compile against libstdc++ 4.9's alloc_traits.h, when
>> that header comes from a module, with an error message along the lines of:
>> .../bits/alloc_traits.h:197:2: error: too few template arguments for
>> class template '__alloctr_rebind'
>>         using rebind_alloc = typename __alloctr_rebind<_Alloc,
>> _Tp>::__type;
>>
>> I think this is enough for you to debug, but let me know if not. I'm
>> going to revert this temporarily to unbreak our modules builds using this
>> version of libstdc++.
>>
>> On Tue, Apr 18, 2017 at 8:14 PM Chandler Carruth <chandlerc at google.com>
>> wrote:
>>
>>> This appears to break a pretty straightforward use of things like
>>> libstdc++'s __alloctr_rebind (undefined template with a default argument,
>>> specializations, etc):
>>> https://github.com/gcc-mirror/gcc/blob/gcc-4_9-branch/
>>> libstdc++-v3/include/bits/alloc_traits.h#L58-L73
>>>
>>> At least, I'm getting errors from this. I'm working on a test case, but
>>> as usual, the test case may be... large.
>>>
>>> On Tue, Apr 18, 2017 at 6:49 PM Richard Smith via cfe-commits <
>>> cfe-commits at lists.llvm.org> wrote:
>>>
>>>> Author: rsmith
>>>> Date: Tue Apr 18 20:36:43 2017
>>>> New Revision: 300650
>>>>
>>>> URL: http://llvm.org/viewvc/llvm-project?rev=300650&view=rev
>>>> Log:
>>>> [modules] Properly look up the owning module for an instantiation of a
>>>> merged template.
>>>>
>>>> When looking for the template instantiation pattern of a templated
>>>> entity,
>>>> consistently select the definition of the pattern if there is one. This
>>>> means
>>>> we'll pick the same owning module when we start instantiating a
>>>> template that
>>>> we'll later pick when determining which modules are visible during that
>>>> instantiation.
>>>>
>>>> Modified:
>>>>     cfe/trunk/lib/AST/Decl.cpp
>>>>     cfe/trunk/lib/AST/DeclCXX.cpp
>>>>     cfe/trunk/lib/Sema/SemaLookup.cpp
>>>>     cfe/trunk/test/Modules/Inputs/template-default-args/a.h
>>>>     cfe/trunk/test/Modules/template-default-args.cpp
>>>>
>>>> Modified: cfe/trunk/lib/AST/Decl.cpp
>>>> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/
>>>> Decl.cpp?rev=300650&r1=300649&r2=300650&view=diff
>>>> ============================================================
>>>> ==================
>>>> --- cfe/trunk/lib/AST/Decl.cpp (original)
>>>> +++ cfe/trunk/lib/AST/Decl.cpp Tue Apr 18 20:36:43 2017
>>>> @@ -2251,6 +2251,13 @@ bool VarDecl::checkInitIsICE() const {
>>>>    return Eval->IsICE;
>>>>  }
>>>>
>>>> +template<typename DeclT>
>>>> +static DeclT *getDefinitionOrSelf(DeclT *D) {
>>>> +  if (auto *Def = D->getDefinition())
>>>> +    return Def;
>>>> +  return D;
>>>> +}
>>>> +
>>>>  VarDecl *VarDecl::getTemplateInstantiationPattern() const {
>>>>    // If it's a variable template specialization, find the template or
>>>> partial
>>>>    // specialization from which it was instantiated.
>>>> @@ -2262,7 +2269,7 @@ VarDecl *VarDecl::getTemplateInstantiati
>>>>            break;
>>>>          VTD = NewVTD;
>>>>        }
>>>> -      return VTD->getTemplatedDecl()->getDefinition();
>>>> +      return getDefinitionOrSelf(VTD->getTemplatedDecl());
>>>>      }
>>>>      if (auto *VTPSD =
>>>>              From.dyn_cast<VarTemplatePartialSpecializationDecl *>()) {
>>>> @@ -2271,7 +2278,7 @@ VarDecl *VarDecl::getTemplateInstantiati
>>>>            break;
>>>>          VTPSD = NewVTPSD;
>>>>        }
>>>> -      return VTPSD->getDefinition();
>>>> +      return getDefinitionOrSelf<VarDecl>(VTPSD);
>>>>      }
>>>>    }
>>>>
>>>> @@ -2280,23 +2287,18 @@ VarDecl *VarDecl::getTemplateInstantiati
>>>>        VarDecl *VD = getInstantiatedFromStaticDataMember();
>>>>        while (auto *NewVD = VD->getInstantiatedFromStaticDataMember())
>>>>          VD = NewVD;
>>>> -      return VD->getDefinition();
>>>> +      return getDefinitionOrSelf(VD);
>>>>      }
>>>>    }
>>>>
>>>>    if (VarTemplateDecl *VarTemplate = getDescribedVarTemplate()) {
>>>> -
>>>>      while (VarTemplate->getInstantiatedFromMemberTemplate()) {
>>>>        if (VarTemplate->isMemberSpecialization())
>>>>          break;
>>>>        VarTemplate = VarTemplate->getInstantiatedFromMemberTemplate();
>>>>      }
>>>>
>>>> -    assert((!VarTemplate->getTemplatedDecl() ||
>>>> -            !isTemplateInstantiation(getTemplateSpecializationKind()))
>>>> &&
>>>> -           "couldn't find pattern for variable instantiation");
>>>> -
>>>> -    return VarTemplate->getTemplatedDecl();
>>>> +    return getDefinitionOrSelf(VarTemplate->getTemplatedDecl());
>>>>    }
>>>>    return nullptr;
>>>>  }
>>>> @@ -3201,7 +3203,7 @@ bool FunctionDecl::isTemplateInstantiati
>>>>  FunctionDecl *FunctionDecl::getTemplateInstantiationPattern() const {
>>>>    // Handle class scope explicit specialization special case.
>>>>    if (getTemplateSpecializationKind() == TSK_ExplicitSpecialization)
>>>> -    return getClassScopeSpecializationPattern();
>>>> +    return getDefinitionOrSelf(getClassScopeSpecializationPattern());
>>>>
>>>>    // If this is a generic lambda call operator specialization, its
>>>>    // instantiation pattern is always its primary template's pattern
>>>> @@ -3214,16 +3216,10 @@ FunctionDecl *FunctionDecl::getTemplateI
>>>>
>>>>    if (isGenericLambdaCallOperatorSpecialization(
>>>>            dyn_cast<CXXMethodDecl>(this))) {
>>>> -    assert(getPrimaryTemplate() && "A generic lambda specialization
>>>> must be "
>>>> -                                   "generated from a primary call
>>>> operator "
>>>> -                                   "template");
>>>> -    assert(getPrimaryTemplate()->getTemplatedDecl()->getBody() &&
>>>> -           "A generic lambda call operator template must always have a
>>>> body - "
>>>> -           "even if instantiated from a prototype (i.e. as written)
>>>> member "
>>>> -           "template");
>>>> -    return getPrimaryTemplate()->getTemplatedDecl();
>>>> +    assert(getPrimaryTemplate() && "not a generic lambda call
>>>> operator?");
>>>> +    return getDefinitionOrSelf(getPrimaryTemplate()->
>>>> getTemplatedDecl());
>>>>    }
>>>> -
>>>> +
>>>>    if (FunctionTemplateDecl *Primary = getPrimaryTemplate()) {
>>>>      while (Primary->getInstantiatedFromMemberTemplate()) {
>>>>        // If we have hit a point where the user provided a
>>>> specialization of
>>>> @@ -3232,11 +3228,14 @@ FunctionDecl *FunctionDecl::getTemplateI
>>>>          break;
>>>>        Primary = Primary->getInstantiatedFromMemberTemplate();
>>>>      }
>>>> -
>>>> -    return Primary->getTemplatedDecl();
>>>> +
>>>> +    return getDefinitionOrSelf(Primary->getTemplatedDecl());
>>>>    }
>>>> -
>>>> -  return getInstantiatedFromMemberFunction();
>>>> +
>>>> +  if (auto *MFD = getInstantiatedFromMemberFunction())
>>>> +    return getDefinitionOrSelf(MFD);
>>>> +
>>>> +  return nullptr;
>>>>  }
>>>>
>>>>  FunctionTemplateDecl *FunctionDecl::getPrimaryTemplate() const {
>>>> @@ -3780,7 +3779,7 @@ EnumDecl *EnumDecl::getTemplateInstantia
>>>>        EnumDecl *ED = getInstantiatedFromMemberEnum();
>>>>        while (auto *NewED = ED->getInstantiatedFromMemberEnum())
>>>>          ED = NewED;
>>>> -      return ED;
>>>> +      return getDefinitionOrSelf(ED);
>>>>      }
>>>>    }
>>>>
>>>>
>>>> Modified: cfe/trunk/lib/AST/DeclCXX.cpp
>>>> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/
>>>> DeclCXX.cpp?rev=300650&r1=300649&r2=300650&view=diff
>>>> ============================================================
>>>> ==================
>>>> --- cfe/trunk/lib/AST/DeclCXX.cpp (original)
>>>> +++ cfe/trunk/lib/AST/DeclCXX.cpp Tue Apr 18 20:36:43 2017
>>>> @@ -1364,6 +1364,13 @@ CXXRecordDecl::setTemplateSpecialization
>>>>  }
>>>>
>>>>  const CXXRecordDecl *CXXRecordDecl::getTemplateInstantiationPattern()
>>>> const {
>>>> +  auto GetDefinitionOrSelf =
>>>> +      [](const CXXRecordDecl *D) -> const CXXRecordDecl * {
>>>> +    if (auto *Def = D->getDefinition())
>>>> +      return Def;
>>>> +    return D;
>>>> +  };
>>>> +
>>>>    // If it's a class template specialization, find the template or
>>>> partial
>>>>    // specialization from which it was instantiated.
>>>>    if (auto *TD = dyn_cast<ClassTemplateSpecializationDecl>(this)) {
>>>> @@ -1374,7 +1381,7 @@ const CXXRecordDecl *CXXRecordDecl::getT
>>>>            break;
>>>>          CTD = NewCTD;
>>>>        }
>>>> -      return CTD->getTemplatedDecl()->getDefinition();
>>>> +      return GetDefinitionOrSelf(CTD->getTemplatedDecl());
>>>>      }
>>>>      if (auto *CTPSD =
>>>>              From.dyn_cast<ClassTemplatePartialSpecializationDecl
>>>> *>()) {
>>>> @@ -1383,7 +1390,7 @@ const CXXRecordDecl *CXXRecordDecl::getT
>>>>            break;
>>>>          CTPSD = NewCTPSD;
>>>>        }
>>>> -      return CTPSD->getDefinition();
>>>> +      return GetDefinitionOrSelf(CTPSD);
>>>>      }
>>>>    }
>>>>
>>>> @@ -1392,7 +1399,7 @@ const CXXRecordDecl *CXXRecordDecl::getT
>>>>        const CXXRecordDecl *RD = this;
>>>>        while (auto *NewRD = RD->getInstantiatedFromMemberClass())
>>>>          RD = NewRD;
>>>> -      return RD->getDefinition();
>>>> +      return GetDefinitionOrSelf(RD);
>>>>      }
>>>>    }
>>>>
>>>>
>>>> Modified: cfe/trunk/lib/Sema/SemaLookup.cpp
>>>> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/
>>>> SemaLookup.cpp?rev=300650&r1=300649&r2=300650&view=diff
>>>> ============================================================
>>>> ==================
>>>> --- cfe/trunk/lib/Sema/SemaLookup.cpp (original)
>>>> +++ cfe/trunk/lib/Sema/SemaLookup.cpp Tue Apr 18 20:36:43 2017
>>>> @@ -1326,12 +1326,6 @@ bool Sema::CppLookupName(LookupResult &R
>>>>    return !R.empty();
>>>>  }
>>>>
>>>> -/// \brief Find the declaration that a class temploid member
>>>> specialization was
>>>> -/// instantiated from, or the member itself if it is an explicit
>>>> specialization.
>>>> -static Decl *getInstantiatedFrom(Decl *D, MemberSpecializationInfo
>>>> *MSInfo) {
>>>> -  return MSInfo->isExplicitSpecialization() ? D :
>>>> MSInfo->getInstantiatedFrom();
>>>> -}
>>>> -
>>>>  Module *Sema::getOwningModule(Decl *Entity) {
>>>>    // If it's imported, grab its owning module.
>>>>    Module *M = Entity->getImportedOwningModule();
>>>> @@ -1413,20 +1407,14 @@ static Module *getDefiningModule(Sema &S
>>>>      if (CXXRecordDecl *Pattern = RD->getTemplateInstantiationPatter
>>>> n())
>>>>        Entity = Pattern;
>>>>    } else if (EnumDecl *ED = dyn_cast<EnumDecl>(Entity)) {
>>>> -    if (MemberSpecializationInfo *MSInfo = ED->
>>>> getMemberSpecializationInfo())
>>>> -      Entity = getInstantiatedFrom(ED, MSInfo);
>>>> +    if (auto *Pattern = ED->getTemplateInstantiationPattern())
>>>> +      Entity = Pattern;
>>>>    } else if (VarDecl *VD = dyn_cast<VarDecl>(Entity)) {
>>>> -    // FIXME: Map from variable template specializations back to the
>>>> template.
>>>> -    if (MemberSpecializationInfo *MSInfo = VD->
>>>> getMemberSpecializationInfo())
>>>> -      Entity = getInstantiatedFrom(VD, MSInfo);
>>>> +    if (VarDecl *Pattern = VD->getTemplateInstantiationPattern())
>>>> +      Entity = Pattern;
>>>>    }
>>>>
>>>> -  // Walk up to the containing context. That might also have been
>>>> instantiated
>>>> -  // from a template.
>>>> -  DeclContext *Context = Entity->getDeclContext();
>>>> -  if (Context->isFileContext())
>>>> -    return S.getOwningModule(Entity);
>>>> -  return getDefiningModule(S, cast<Decl>(Context));
>>>> +  return S.getOwningModule(Entity);
>>>>  }
>>>>
>>>>  llvm::DenseSet<Module*> &Sema::getLookupModules() {
>>>>
>>>> Modified: cfe/trunk/test/Modules/Inputs/template-default-args/a.h
>>>> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/
>>>> Modules/Inputs/template-default-args/a.h?rev=300650&
>>>> r1=300649&r2=300650&view=diff
>>>> ============================================================
>>>> ==================
>>>> --- cfe/trunk/test/Modules/Inputs/template-default-args/a.h (original)
>>>> +++ cfe/trunk/test/Modules/Inputs/template-default-args/a.h Tue Apr 18
>>>> 20:36:43 2017
>>>> @@ -14,3 +14,11 @@ struct FriendL {
>>>>    template<typename T> friend struct L;
>>>>  };
>>>>  END
>>>> +
>>>> +namespace DeferredLookup {
>>>> +  template<typename T, typename U = T> using X = U;
>>>> +  template<typename T> void f() { (void) X<T>(); }
>>>> +  template<typename T> int n = X<T>();
>>>> +  template<typename T> struct S { X<T> xt; enum E : int; };
>>>> +  template<typename T> enum S<T>::E : int { a = X<T>() };
>>>> +}
>>>>
>>>> Modified: cfe/trunk/test/Modules/template-default-args.cpp
>>>> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/
>>>> Modules/template-default-args.cpp?rev=300650&r1=300649&r2=
>>>> 300650&view=diff
>>>> ============================================================
>>>> ==================
>>>> --- cfe/trunk/test/Modules/template-default-args.cpp (original)
>>>> +++ cfe/trunk/test/Modules/template-default-args.cpp Tue Apr 18
>>>> 20:36:43 2017
>>>> @@ -44,3 +44,18 @@ H<> h; // expected-error {{default argum
>>>>  I<> i;
>>>>  L<> *l;
>>>>  END
>>>> +
>>>> +namespace DeferredLookup {
>>>> +  template<typename T, typename U = T> using X = U;
>>>> +  template<typename T> void f() { (void) X<T>(); }
>>>> +  template<typename T> int n = X<T>(); // expected-warning
>>>> {{extension}}
>>>> +  template<typename T> struct S { X<T> xt; enum E : int; };
>>>> +  template<typename T> enum S<T>::E : int { a = X<T>() };
>>>> +
>>>> +  void test() {
>>>> +    f<int>();
>>>> +    n<int> = 1;
>>>> +    S<int> s;
>>>> +    S<int>::E e = S<int>::E::a;
>>>> +  }
>>>> +}
>>>>
>>>>
>>>> _______________________________________________
>>>> cfe-commits mailing list
>>>> cfe-commits at lists.llvm.org
>>>> http://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/20170418/11dbdc55/attachment-0001.html>


More information about the cfe-commits mailing list