From 0650f897a470730159d70e1617c9697c65b514d4 Mon Sep 17 00:00:00 2001 From: Francis Visoiu Mistrih Date: Fri, 15 Feb 2019 03:06:15 +0000 Subject: [PATCH] Revert "Fix implementation of [temp.local]p4." This reverts commit 40bd10b770813bd1471d46f514545437516aa4ba. This seems to now emit an error when building the sanitizer tests: http://green.lab.llvm.org/green/job/clang-stage1-configure-RA/53965/consoleFull. llvm-svn: 354097 --- clang/include/clang/Sema/Lookup.h | 19 +-- clang/include/clang/Sema/Sema.h | 16 +-- clang/lib/Sema/SemaDecl.cpp | 18 ++- clang/lib/Sema/SemaDeclCXX.cpp | 1 + clang/lib/Sema/SemaLookup.cpp | 22 +--- clang/lib/Sema/SemaTemplate.cpp | 142 +++++++++------------- clang/test/CXX/class.access/p4.cpp | 10 +- clang/test/CXX/temp/temp.decls/temp.friend/p1.cpp | 6 +- clang/test/SemaTemplate/temp.cpp | 39 +----- 9 files changed, 89 insertions(+), 184 deletions(-) diff --git a/clang/include/clang/Sema/Lookup.h b/clang/include/clang/Sema/Lookup.h index 0466d06..990005f 100644 --- a/clang/include/clang/Sema/Lookup.h +++ b/clang/include/clang/Sema/Lookup.h @@ -172,8 +172,7 @@ public: : SemaPtr(Other.SemaPtr), NameInfo(Other.NameInfo), LookupKind(Other.LookupKind), IDNS(Other.IDNS), Redecl(Other.Redecl), ExternalRedecl(Other.ExternalRedecl), HideTags(Other.HideTags), - AllowHidden(Other.AllowHidden), - TemplateNameLookup(Other.TemplateNameLookup) {} + AllowHidden(Other.AllowHidden) {} // FIXME: Remove these deleted methods once the default build includes // -Wdeprecated. @@ -194,8 +193,7 @@ public: HideTags(std::move(Other.HideTags)), Diagnose(std::move(Other.Diagnose)), AllowHidden(std::move(Other.AllowHidden)), - Shadowed(std::move(Other.Shadowed)), - TemplateNameLookup(std::move(Other.TemplateNameLookup)) { + Shadowed(std::move(Other.Shadowed)) { Other.Paths = nullptr; Other.Diagnose = false; } @@ -218,7 +216,6 @@ public: Diagnose = std::move(Other.Diagnose); AllowHidden = std::move(Other.AllowHidden); Shadowed = std::move(Other.Shadowed); - TemplateNameLookup = std::move(Other.TemplateNameLookup); Other.Paths = nullptr; Other.Diagnose = false; return *this; @@ -289,15 +286,6 @@ public: HideTags = Hide; } - /// Sets whether this is a template-name lookup. For template-name lookups, - /// injected-class-names are treated as naming a template rather than a - /// template specialization. - void setTemplateNameLookup(bool TemplateName) { - TemplateNameLookup = TemplateName; - } - - bool isTemplateNameLookup() const { return TemplateNameLookup; } - bool isAmbiguous() const { return getResultKind() == Ambiguous; } @@ -751,9 +739,6 @@ private: /// declaration that we skipped. This only happens when \c LookupKind /// is \c LookupRedeclarationWithLinkage. bool Shadowed = false; - - /// True if we're looking up a template-name. - bool TemplateNameLookup = false; }; /// Consumes visible declarations found when searching for diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h index 1247b24..10eff53 100644 --- a/clang/include/clang/Sema/Sema.h +++ b/clang/include/clang/Sema/Sema.h @@ -6212,21 +6212,9 @@ public: // C++ Templates [C++ 14] // void FilterAcceptableTemplateNames(LookupResult &R, - bool AllowFunctionTemplates = true, - bool AllowDependent = true); + bool AllowFunctionTemplates = true); bool hasAnyAcceptableTemplateNames(LookupResult &R, - bool AllowFunctionTemplates = true, - bool AllowDependent = true); - /// Try to interpret the lookup result D as a template-name. - /// - /// \param D A declaration found by name lookup. - /// \param AllowFunctionTemplates Whether function templates should be - /// considered valid results. - /// \param AllowDependent Whether unresolved using declarations (that might - /// name templates) should be considered valid results. - NamedDecl *getAsTemplateNameDecl(NamedDecl *D, - bool AllowFunctionTemplates = true, - bool AllowDependent = true); + bool AllowFunctionTemplates = true); bool LookupTemplateName(LookupResult &R, Scope *S, CXXScopeSpec &SS, QualType ObjectType, bool EnteringContext, diff --git a/clang/lib/Sema/SemaDecl.cpp b/clang/lib/Sema/SemaDecl.cpp index d6e44af..a9e6eb1 100644 --- a/clang/lib/Sema/SemaDecl.cpp +++ b/clang/lib/Sema/SemaDecl.cpp @@ -1017,8 +1017,7 @@ Corrected: case LookupResult::Ambiguous: if (getLangOpts().CPlusPlus && NextToken.is(tok::less) && - hasAnyAcceptableTemplateNames(Result, /*AllowFunctionTemplates=*/true, - /*AllowDependent=*/false)) { + hasAnyAcceptableTemplateNames(Result)) { // C++ [temp.local]p3: // A lookup that finds an injected-class-name (10.2) can result in an // ambiguity in certain cases (for example, if it is found in more than @@ -1042,9 +1041,7 @@ Corrected: } if (getLangOpts().CPlusPlus && NextToken.is(tok::less) && - (IsFilteredTemplateName || - hasAnyAcceptableTemplateNames(Result, /*AllowFunctionTemplates=*/true, - /*AllowDependent=*/false))) { + (IsFilteredTemplateName || hasAnyAcceptableTemplateNames(Result))) { // C++ [temp.names]p3: // After name lookup (3.4) finds that a name is a template-name or that // an operator-function-id or a literal- operator-id refers to a set of @@ -1063,16 +1060,15 @@ Corrected: Template = Context.getOverloadedTemplateName(Result.begin(), Result.end()); } else { - auto *TD = cast(getAsTemplateNameDecl( - *Result.begin(), /*AllowFunctionTemplates=*/true, - /*AllowDependent=*/false)); + TemplateDecl *TD + = cast((*Result.begin())->getUnderlyingDecl()); IsFunctionTemplate = isa(TD); IsVarTemplate = isa(TD); if (SS.isSet() && !SS.isInvalid()) - Template = - Context.getQualifiedTemplateName(SS.getScopeRep(), - /*TemplateKeyword=*/false, TD); + Template = Context.getQualifiedTemplateName(SS.getScopeRep(), + /*TemplateKeyword=*/false, + TD); else Template = TemplateName(TD); } diff --git a/clang/lib/Sema/SemaDeclCXX.cpp b/clang/lib/Sema/SemaDeclCXX.cpp index 7457d1c..8badbbd 100644 --- a/clang/lib/Sema/SemaDeclCXX.cpp +++ b/clang/lib/Sema/SemaDeclCXX.cpp @@ -1128,6 +1128,7 @@ static bool checkTupleLikeDecomposition(Sema &S, } } } + S.FilterAcceptableTemplateNames(MemberGet); } unsigned I = 0; diff --git a/clang/lib/Sema/SemaLookup.cpp b/clang/lib/Sema/SemaLookup.cpp index 86960e0..249be78 100644 --- a/clang/lib/Sema/SemaLookup.cpp +++ b/clang/lib/Sema/SemaLookup.cpp @@ -2172,27 +2172,11 @@ bool Sema::LookupQualifiedName(LookupResult &R, DeclContext *LookupCtx, DeclContext::lookup_iterator FirstD = FirstPath->Decls.begin(); DeclContext::lookup_iterator CurrentD = Path->Decls.begin(); - // Get the decl that we should use for deduplicating this lookup. - auto GetRepresentativeDecl = [&](NamedDecl *D) -> Decl * { - // C++ [temp.local]p3: - // A lookup that finds an injected-class-name (10.2) can result in - // an ambiguity in certain cases (for example, if it is found in - // more than one base class). If all of the injected-class-names - // that are found refer to specializations of the same class - // template, and if the name is used as a template-name, the - // reference refers to the class template itself and not a - // specialization thereof, and is not ambiguous. - if (R.isTemplateNameLookup()) - if (auto *TD = getAsTemplateNameDecl(D)) - D = TD; - return D->getUnderlyingDecl()->getCanonicalDecl(); - }; - while (FirstD != FirstPath->Decls.end() && CurrentD != Path->Decls.end()) { - if (GetRepresentativeDecl(*FirstD) != - GetRepresentativeDecl(*CurrentD)) - break; + if ((*FirstD)->getUnderlyingDecl()->getCanonicalDecl() != + (*CurrentD)->getUnderlyingDecl()->getCanonicalDecl()) + break; ++FirstD; ++CurrentD; diff --git a/clang/lib/Sema/SemaTemplate.cpp b/clang/lib/Sema/SemaTemplate.cpp index 3f642b5..2e73096 100644 --- a/clang/lib/Sema/SemaTemplate.cpp +++ b/clang/lib/Sema/SemaTemplate.cpp @@ -66,20 +66,17 @@ static Expr *clang::formAssociatedConstraints(TemplateParameterList *Params, /// Determine whether the declaration found is acceptable as the name /// of a template and, if so, return that template declaration. Otherwise, -/// returns null. -/// -/// Note that this may return an UnresolvedUsingValueDecl if AllowDependent -/// is true. In all other cases it will return a TemplateDecl (or null). -NamedDecl *Sema::getAsTemplateNameDecl(NamedDecl *D, - bool AllowFunctionTemplates, - bool AllowDependent) { - D = D->getUnderlyingDecl(); +/// returns NULL. +static NamedDecl *isAcceptableTemplateName(ASTContext &Context, + NamedDecl *Orig, + bool AllowFunctionTemplates) { + NamedDecl *D = Orig->getUnderlyingDecl(); if (isa(D)) { if (!AllowFunctionTemplates && isa(D)) return nullptr; - return D; + return Orig; } if (CXXRecordDecl *Record = dyn_cast(D)) { @@ -110,29 +107,54 @@ NamedDecl *Sema::getAsTemplateNameDecl(NamedDecl *D, // 'using Dependent::foo;' can resolve to a template name. // 'using typename Dependent::foo;' cannot (not even if 'foo' is an // injected-class-name). - if (AllowDependent && isa(D)) + if (isa(D)) return D; return nullptr; } void Sema::FilterAcceptableTemplateNames(LookupResult &R, - bool AllowFunctionTemplates, - bool AllowDependent) { + bool AllowFunctionTemplates) { + // The set of class templates we've already seen. + llvm::SmallPtrSet ClassTemplates; LookupResult::Filter filter = R.makeFilter(); while (filter.hasNext()) { NamedDecl *Orig = filter.next(); - if (!getAsTemplateNameDecl(Orig, AllowFunctionTemplates, AllowDependent)) + NamedDecl *Repl = isAcceptableTemplateName(Context, Orig, + AllowFunctionTemplates); + if (!Repl) filter.erase(); + else if (Repl != Orig) { + + // C++ [temp.local]p3: + // A lookup that finds an injected-class-name (10.2) can result in an + // ambiguity in certain cases (for example, if it is found in more than + // one base class). If all of the injected-class-names that are found + // refer to specializations of the same class template, and if the name + // is used as a template-name, the reference refers to the class + // template itself and not a specialization thereof, and is not + // ambiguous. + if (ClassTemplateDecl *ClassTmpl = dyn_cast(Repl)) + if (!ClassTemplates.insert(ClassTmpl).second) { + filter.erase(); + continue; + } + + // FIXME: we promote access to public here as a workaround to + // the fact that LookupResult doesn't let us remember that we + // found this template through a particular injected class name, + // which means we end up doing nasty things to the invariants. + // Pretending that access is public is *much* safer. + filter.replace(Repl, AS_public); + } } filter.done(); } bool Sema::hasAnyAcceptableTemplateNames(LookupResult &R, - bool AllowFunctionTemplates, - bool AllowDependent) { + bool AllowFunctionTemplates) { for (LookupResult::iterator I = R.begin(), IEnd = R.end(); I != IEnd; ++I) - if (getAsTemplateNameDecl(*I, AllowFunctionTemplates, AllowDependent)) + if (isAcceptableTemplateName(Context, *I, AllowFunctionTemplates)) return true; return false; @@ -176,45 +198,20 @@ TemplateNameKind Sema::isTemplateName(Scope *S, MemberOfUnknownSpecialization)) return TNK_Non_template; if (R.empty()) return TNK_Non_template; - - NamedDecl *D = nullptr; if (R.isAmbiguous()) { - // If we got an ambiguity involving a non-function template, treat this - // as a template name, and pick an arbitrary template for error recovery. - bool AnyFunctionTemplates = false; - for (NamedDecl *FoundD : R) { - if (NamedDecl *FoundTemplate = getAsTemplateNameDecl(FoundD)) { - if (isa(FoundTemplate)) - AnyFunctionTemplates = true; - else { - D = FoundTemplate; - break; - } - } - } - - // If we didn't find any templates at all, this isn't a template name. - // Leave the ambiguity for a later lookup to diagnose. - if (!D && !AnyFunctionTemplates) { - R.suppressDiagnostics(); - return TNK_Non_template; - } + // Suppress diagnostics; we'll redo this lookup later. + R.suppressDiagnostics(); - // If the only templates were function templates, filter out the rest. - // We'll diagnose the ambiguity later. - if (!D) - FilterAcceptableTemplateNames(R); + // FIXME: we might have ambiguous templates, in which case we + // should at least parse them properly! + return TNK_Non_template; } - // At this point, we have either picked a single template name declaration D - // or we have a non-empty set of results R containing either one template name - // declaration or a set of function templates. - TemplateName Template; TemplateNameKind TemplateKind; unsigned ResultCount = R.end() - R.begin(); - if (!D && ResultCount > 1) { + if (ResultCount > 1) { // We assume that we'll preserve the qualifier from a function // template name in other ways. Template = Context.getOverloadedTemplateName(R.begin(), R.end()); @@ -222,19 +219,12 @@ TemplateNameKind Sema::isTemplateName(Scope *S, // We'll do this lookup again later. R.suppressDiagnostics(); + } else if (isa((*R.begin())->getUnderlyingDecl())) { + // We don't yet know whether this is a template-name or not. + MemberOfUnknownSpecialization = true; + return TNK_Non_template; } else { - if (!D) { - D = getAsTemplateNameDecl(*R.begin()); - assert(D && "unambiguous result is not a template name"); - } - - if (isa(D)) { - // We don't yet know whether this is a template-name or not. - MemberOfUnknownSpecialization = true; - return TNK_Non_template; - } - - TemplateDecl *TD = cast(D); + TemplateDecl *TD = cast((*R.begin())->getUnderlyingDecl()); if (SS.isSet() && !SS.isInvalid()) { NestedNameSpecifier *Qualifier = SS.getScopeRep(); @@ -326,8 +316,6 @@ bool Sema::LookupTemplateName(LookupResult &Found, bool EnteringContext, bool &MemberOfUnknownSpecialization, SourceLocation TemplateKWLoc) { - Found.setTemplateNameLookup(true); - // Determine where to perform name lookup MemberOfUnknownSpecialization = false; DeclContext *LookupCtx = nullptr; @@ -402,9 +390,6 @@ bool Sema::LookupTemplateName(LookupResult &Found, IsDependent |= Found.wasNotFoundInCurrentInstantiation(); } - if (Found.isAmbiguous()) - return false; - if (Found.empty() && !IsDependent) { // If we did not find any names, attempt to correct any typos. DeclarationName Name = Found.getLookupName(); @@ -422,9 +407,7 @@ bool Sema::LookupTemplateName(LookupResult &Found, if (auto *ND = Corrected.getFoundDecl()) Found.addDecl(ND); FilterAcceptableTemplateNames(Found); - if (Found.isAmbiguous()) { - Found.clear(); - } else if (!Found.empty()) { + if (!Found.empty()) { if (LookupCtx) { std::string CorrectedStr(Corrected.getAsString(getLangOpts())); bool DroppedSpecifier = Corrected.WillReplaceSpecifier() && @@ -474,19 +457,14 @@ bool Sema::LookupTemplateName(LookupResult &Found, // Note: C++11 does not perform this second lookup. LookupResult FoundOuter(*this, Found.getLookupName(), Found.getNameLoc(), LookupOrdinaryName); - FoundOuter.setTemplateNameLookup(true); LookupName(FoundOuter, S); - // FIXME: We silently accept an ambiguous lookup here, in violation of - // [basic.lookup]/1. FilterAcceptableTemplateNames(FoundOuter, /*AllowFunctionTemplates=*/false); - NamedDecl *OuterTemplate; if (FoundOuter.empty()) { // - if the name is not found, the name found in the class of the // object expression is used, otherwise - } else if (FoundOuter.isAmbiguous() || !FoundOuter.isSingleResult() || - !(OuterTemplate = - getAsTemplateNameDecl(FoundOuter.getFoundDecl()))) { + } else if (!FoundOuter.getAsSingle() || + FoundOuter.isAmbiguous()) { // - if the name is found in the context of the entire // postfix-expression and does not name a class template, the name // found in the class of the object expression is used, otherwise @@ -496,8 +474,8 @@ bool Sema::LookupTemplateName(LookupResult &Found, // entity as the one found in the class of the object expression, // otherwise the program is ill-formed. if (!Found.isSingleResult() || - getAsTemplateNameDecl(Found.getFoundDecl())->getCanonicalDecl() != - OuterTemplate->getCanonicalDecl()) { + Found.getFoundDecl()->getCanonicalDecl() + != FoundOuter.getFoundDecl()->getCanonicalDecl()) { Diag(Found.getNameLoc(), diag::ext_nested_name_member_ref_lookup_ambiguous) << Found.getLookupName() @@ -567,8 +545,7 @@ void Sema::diagnoseExprIntendedAsTemplateName(Scope *S, ExprResult TemplateName, // Try to correct the name by looking for templates and C++ named casts. struct TemplateCandidateFilter : CorrectionCandidateCallback { - Sema &S; - TemplateCandidateFilter(Sema &S) : S(S) { + TemplateCandidateFilter() { WantTypeSpecifiers = false; WantExpressionKeywords = false; WantRemainingKeywords = false; @@ -576,7 +553,7 @@ void Sema::diagnoseExprIntendedAsTemplateName(Scope *S, ExprResult TemplateName, }; bool ValidateCandidate(const TypoCorrection &Candidate) override { if (auto *ND = Candidate.getCorrectionDecl()) - return S.getAsTemplateNameDecl(ND); + return isAcceptableTemplateName(ND->getASTContext(), ND, true); return Candidate.isKeyword(); } }; @@ -584,11 +561,12 @@ void Sema::diagnoseExprIntendedAsTemplateName(Scope *S, ExprResult TemplateName, DeclarationName Name = NameInfo.getName(); if (TypoCorrection Corrected = CorrectTypo(NameInfo, LookupKind, S, &SS, - llvm::make_unique(*this), + llvm::make_unique(), CTK_ErrorRecovery, LookupCtx)) { auto *ND = Corrected.getFoundDecl(); if (ND) - ND = getAsTemplateNameDecl(ND); + ND = isAcceptableTemplateName(Context, ND, + /*AllowFunctionTemplates*/ true); if (ND || Corrected.isKeyword()) { if (LookupCtx) { std::string CorrectedStr(Corrected.getAsString(getLangOpts())); @@ -4284,7 +4262,7 @@ TemplateNameKind Sema::ActOnDependentTemplateName(Scope *S, LookupOrdinaryName); bool MOUS; if (!LookupTemplateName(R, S, SS, ObjectType.get(), EnteringContext, - MOUS, TemplateKWLoc) && !R.isAmbiguous()) + MOUS, TemplateKWLoc)) Diag(Name.getBeginLoc(), diag::err_no_member) << DNI.getName() << LookupCtx << SS.getRange(); return TNK_Non_template; diff --git a/clang/test/CXX/class.access/p4.cpp b/clang/test/CXX/class.access/p4.cpp index a2d0da1..6d452d8 100644 --- a/clang/test/CXX/class.access/p4.cpp +++ b/clang/test/CXX/class.access/p4.cpp @@ -514,12 +514,16 @@ namespace test17 { } namespace test18 { - template class A {}; // expected-note {{member is declared here}} - class B : A { // expected-note {{constrained by implicitly private inheritance here}} + template class A {}; + class B : A { A member; }; + + // FIXME: this access to A should be forbidden (because C++ is dumb), + // but LookupResult can't express the necessary information to do + // the check, so we aggressively suppress access control. class C : B { - A member; // expected-error {{'A' is a private member of 'test18::A'}} + A member; }; } diff --git a/clang/test/CXX/temp/temp.decls/temp.friend/p1.cpp b/clang/test/CXX/temp/temp.decls/temp.friend/p1.cpp index ab1b9f7..849728a 100644 --- a/clang/test/CXX/temp/temp.decls/temp.friend/p1.cpp +++ b/clang/test/CXX/temp/temp.decls/temp.friend/p1.cpp @@ -380,10 +380,10 @@ template struct A { namespace test18 { namespace ns1 { template struct foo {}; } // expected-note{{candidate ignored: not a function template}} namespace ns2 { void foo() {} } // expected-note{{candidate ignored: not a function template}} -using ns1::foo; // expected-note {{found by name lookup}} -using ns2::foo; // expected-note {{found by name lookup}} +using ns1::foo; +using ns2::foo; template class A { - friend void foo() {} // expected-error {{ambiguous}} expected-error{{no candidate function template was found for dependent friend function template specialization}} + friend void foo() {} // expected-error{{no candidate function template was found for dependent friend function template specialization}} }; } diff --git a/clang/test/SemaTemplate/temp.cpp b/clang/test/SemaTemplate/temp.cpp index a8a2dae..e037f0f 100644 --- a/clang/test/SemaTemplate/temp.cpp +++ b/clang/test/SemaTemplate/temp.cpp @@ -8,43 +8,12 @@ namespace test0 { // PR7252 namespace test1 { - namespace A { template struct Base { typedef T t; }; } // expected-note 3{{member}} + namespace A { template struct Base { typedef T t; }; } // expected-note {{member found}} namespace B { template struct Base { typedef T t; }; } // expected-note {{member found}} template struct Derived : A::Base, B::Base { - typename Derived::Base::t x; // expected-error {{found in multiple base classes of different types}} + // FIXME: the syntax error here is unfortunate + typename Derived::Base::t x; // expected-error {{found in multiple base classes of different types}} \ + // expected-error {{expected member name or ';'}} }; - - class X : A::Base {}; // expected-note 2{{private}} - class Y : A::Base {}; - struct Z : A::Base {}; - struct Use1 : X, Y { - Base b1; // expected-error {{private}} - Use1::Base b2; // expected-error {{private}} - }; - struct Use2 : Z, Y { - Base b1; - Use2::Base b2; - }; - struct Use3 : X, Z { - Base b1; - Use3::Base b2; - }; -} - -namespace test2 { - struct A { static int x; }; // expected-note 4{{member}} - struct B { template static T x(); }; // expected-note 4{{member}} - struct C { template struct x {}; }; // expected-note 3{{member}} - struct D { template static T x(); }; // expected-note {{member}} - - template struct X : T... {}; - - void f() { - X::x(); // expected-error {{found in multiple base classes of different types}} - X::x(); // expected-error {{found in multiple base classes of different types}} - X::x(); // expected-error {{found in multiple base classes of different types}} - X::x(); // expected-error {{found in multiple base classes of different types}} - X::x(); // expected-error {{found in multiple base classes of different types}} - } } -- 2.7.4