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

Chandler Carruth via cfe-commits cfe-commits at lists.llvm.org
Tue Apr 18 22:41:06 PDT 2017


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->getTemplateInstantiationPattern())
>>>        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/20170419/0a020385/attachment-0001.html>


More information about the cfe-commits mailing list