Re-committed with fix in r300938.
On 20 April 2017 at 17:06, Richard Smith <rich...@metafoo.co.uk> wrote:
> On 18 April 2017 at 22:41, Chandler Carruth <chandl...@google.com> wrote:
>
>> Reverted in r300658.
>>
>
> For posterity, the above has an off-by-one error, the revert was r300659.
>
>
>> On Tue, Apr 18, 2017 at 8:30 PM Chandler Carruth <chandl...@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 <chandl...@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/libstd
>>>> c++-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@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(getPrimary
>>>>> Template()->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/DeclCX
>>>>> X.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/SemaL
>>>>> ookup.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->getTemplateInstantiationPa
>>>>> ttern())
>>>>> 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@lists.llvm.org
>>>>> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>>>>>
>>>>
>
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
