Richard, I got it, thanks. Best regards, Alexey Bataev ============= Software Engineer Intel Compiler Team
06.02.2015 0:54, Richard Smith пишет:
On Thu, Feb 5, 2015 at 4:45 AM, Alexey Bataev <[email protected] <mailto:[email protected]>> wrote:Richard, Reid, Thanks for the review! ================ Comment at: lib/AST/Type.cpp:1894-1900 @@ -1893,5 +1893,9 @@ TagDecl *TagType::getDecl() const { return getInterestingTagDecl(decl); } +TagDecl *TagType::getDecl() { + return getInterestingTagDecl(decl); +} + ---------------- rsmith wrote: > What's the purpose of this change? We'll call the `const` version for a non-const object without this, which will do the same thing. Removed. ================ Comment at: lib/Sema/SemaDecl.cpp:132 @@ +131,3 @@ +namespace { +enum BasesLookupResult { NotFound, FoundNotType, FoundType }; +} // namespace ---------------- rsmith wrote: > rnk wrote: > > "FoundNonType" would be closer to the standardese terminology of "non-type template parameter" > This should either be an enum class or should use a prefix for its names; these identifiers seem too general to drop into global scope here. Turned enum into enum class and renamed FoundNotType to FoundNonTypeTemplateParam ================ Comment at: lib/Sema/SemaDecl.cpp:139 @@ +138,3 @@ +/// type decl, \a FoundType if only type decls are found. +static BasesLookupResult lookupInBases(Sema &S, const IdentifierInfo &II, + SourceLocation NameLoc, ---------------- rsmith wrote: > The name of this function should mention something about dependent bases, since that's what's interesting about it and how it differs from normal name lookup. Renamed to lookupUnqualifiedTypeNameInDependentBase ================ Comment at: lib/Sema/SemaDecl.cpp:153 @@ +152,3 @@ + /*LookupKind=*/Sema::LookupAnyName); + if (!S.LookupQualifiedName(LR, DC) && + LR.getResultKind() == LookupResult::NotFound) ---------------- rsmith wrote: > You're going to lengths to ensure we look up into dependent bases of dependent bases, but you don't support finding names in dependent bases of non-dependent bases of dependent bases: we'll use normal name lookup here in that case. Maybe remove this whole case and do the same thing you do below: look up into the RecordDecl you get here and then recurse to its base classes. Hmm, maybe I'm missing something, but is it possible at all? How non-dependent class may have dependent base? The base class may be part of the current instantiation: template<typename T> struct A { typedef int type; }; template<typename T> struct B : A<T> { struct C; }; template<typename T> struct B<T>::C : B { void f() { type x; } };Here, B<T> is a non-dependent base class of B<T>::C, because it is part of the same "current instantiation" (that is, whenever you're instantiating that definition of B<T>::C, you must also have an instantiation of the primary template of B<T>).================ Comment at: lib/Sema/SemaDecl.cpp:159-160 @@ +158,4 @@ + FoundTypeDecl = FoundType; + } + } else if (auto *TST = + Base.getType()->getAs<TemplateSpecializationType>()) { ---------------- rnk wrote: > You can reduce the indentation here with continue. Ok ================ Comment at: lib/Sema/SemaDecl.cpp:173-187 @@ +172,17 @@ + continue; + switch (lookupInBases(S, II, NameLoc, BasePrimaryTemplate)) { + case FoundNotType: + return FoundNotType; + case FoundType: + FoundTypeDecl = FoundType; + break; + case NotFound: + break; + } + for (NamedDecl *ND : BasePrimaryTemplate->lookup(&II)) { + if (!isa<TypeDecl>(ND)) + return FoundNotType; + FoundTypeDecl = FoundType; + } + } + } ---------------- rsmith wrote: > Switch these two over: first look up in the base class itself, and then look up in its base classes if you find nothing. Otherwise you'll get the wrong result if the name is a type in one place and a non-type in the other. Ok, got it. ================ Comment at: lib/Sema/SemaDecl.cpp:196-202 @@ -133,13 +195,9 @@ SourceLocation NameLoc) { // Find the first parent class template context, if any. - // FIXME: Perform the lookup in all enclosing class templates. const CXXRecordDecl *RD = nullptr; for (DeclContext *DC = S.CurContext; DC; DC = DC->getParent()) { RD = dyn_cast<CXXRecordDecl>(DC); if (RD && RD->getDescribedClassTemplate()) break; } // Look for type decls in dependent base classes that have known primary ---------------- rsmith wrote: > If there are multiple such base classes, we should ideally search outer ones if we find nothing in the innermost one, to match normal unqualified lookup. Agree, will be fixed ================ Comment at: test/SemaTemplate/ms-lookup-template-base-classes.cpp:411 @@ -392,2 +410,3 @@ +C<int> c; // expected-note {{in instantiation of template class 'type_in_base_of_multiple_dependent_bases::C<int>' requested here}} } ---------------- rnk wrote: > Can you add a test for the case where we look through non-dependent bases? So far as I can tell, these are all dependent. Test in 'namespace type_in_base_of_dependent_base' checks this already. But I'll add another one. http://reviews.llvm.org/D7173 EMAIL PREFERENCES http://reviews.llvm.org/settings/panel/emailpreferences/
_______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
