diff options
author | Richard Smith <richard-llvm@metafoo.co.uk> | 2011-06-05 22:42:48 +0000 |
---|---|---|
committer | Richard Smith <richard-llvm@metafoo.co.uk> | 2011-06-05 22:42:48 +0000 |
commit | f50e88a793dd5bc7073c717fec78912e3234e95a (patch) | |
tree | 5c3f5d715bfd2623a8f80bd27ee5b61142edb48a | |
parent | 25a857b8039bc86695614126bfe4f21035d6c76b (diff) |
Fix PR10053: Improve diagnostics and error recovery for code which some compilers incorrectly accept due to a lack of proper support for two-phase name lookup.
git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@132672 91177308-0d34-0410-b5e6-96231b3b80d8
-rw-r--r-- | include/clang/Basic/DiagnosticSemaKinds.td | 4 | ||||
-rw-r--r-- | lib/Sema/SemaOverload.cpp | 148 | ||||
-rw-r--r-- | test/SemaTemplate/dependent-names.cpp | 135 | ||||
-rw-r--r-- | test/SemaTemplate/instantiate-call.cpp | 5 | ||||
-rw-r--r-- | www/compatibility.html | 24 |
5 files changed, 295 insertions, 21 deletions
diff --git a/include/clang/Basic/DiagnosticSemaKinds.td b/include/clang/Basic/DiagnosticSemaKinds.td index 83644b618e..e7c9a16972 100644 --- a/include/clang/Basic/DiagnosticSemaKinds.td +++ b/include/clang/Basic/DiagnosticSemaKinds.td @@ -2173,6 +2173,10 @@ def err_unexpected_namespace : Error< def err_undeclared_var_use : Error<"use of undeclared identifier %0">; def note_dependent_var_use : Note<"must qualify identifier to find this " "declaration in dependent base class">; +def err_not_found_by_two_phase_lookup : Error<"call to function %0 that is neither " + "visible in the template definition nor found by argument dependent lookup">; +def note_not_found_by_two_phase_lookup : Note<"%0 should be declared prior to the " + "call site%select{| or in %2| or in an associated namespace of one of its arguments}1">; def err_undeclared_use : Error<"use of undeclared %0">; def warn_deprecated : Warning<"%0 is deprecated">, InGroup<DeprecatedDeclarations>; diff --git a/lib/Sema/SemaOverload.cpp b/lib/Sema/SemaOverload.cpp index eb1400cd57..ca9152e87e 100644 --- a/lib/Sema/SemaOverload.cpp +++ b/lib/Sema/SemaOverload.cpp @@ -7826,6 +7826,111 @@ void Sema::AddOverloadedCallCandidates(UnresolvedLookupExpr *ULE, ULE->isStdAssociatedNamespace()); } +/// Attempt to recover from an ill-formed use of a non-dependent name in a +/// template, where the non-dependent name was declared after the template +/// was defined. This is common in code written for a compilers which do not +/// correctly implement two-stage name lookup. +/// +/// Returns true if a viable candidate was found and a diagnostic was issued. +static bool +DiagnoseTwoPhaseLookup(Sema &SemaRef, SourceLocation FnLoc, + const CXXScopeSpec &SS, LookupResult &R, + TemplateArgumentListInfo *ExplicitTemplateArgs, + Expr **Args, unsigned NumArgs) { + if (SemaRef.ActiveTemplateInstantiations.empty() || !SS.isEmpty()) + return false; + + for (DeclContext *DC = SemaRef.CurContext; DC; DC = DC->getParent()) { + SemaRef.LookupQualifiedName(R, DC); + + if (!R.empty()) { + R.suppressDiagnostics(); + + if (isa<CXXRecordDecl>(DC)) { + // Don't diagnose names we find in classes; we get much better + // diagnostics for these from DiagnoseEmptyLookup. + R.clear(); + return false; + } + + OverloadCandidateSet Candidates(FnLoc); + for (LookupResult::iterator I = R.begin(), E = R.end(); I != E; ++I) + AddOverloadedCallCandidate(SemaRef, I.getPair(), + ExplicitTemplateArgs, Args, NumArgs, + Candidates, false); + + OverloadCandidateSet::iterator Best; + if (Candidates.BestViableFunction(SemaRef, FnLoc, Best) != OR_Success) + // No viable functions. Don't bother the user with notes for functions + // which don't work and shouldn't be found anyway. + return false; + + // Find the namespaces where ADL would have looked, and suggest + // declaring the function there instead. + Sema::AssociatedNamespaceSet AssociatedNamespaces; + Sema::AssociatedClassSet AssociatedClasses; + SemaRef.FindAssociatedClassesAndNamespaces(Args, NumArgs, + AssociatedNamespaces, + AssociatedClasses); + // Never suggest declaring a function within namespace 'std'. + if (DeclContext *Std = SemaRef.getStdNamespace()) { + // Use two passes: SmallPtrSet::erase invalidates too many iterators + // to be used in the loop. + llvm::SmallVector<DeclContext*, 4> StdNamespaces; + for (Sema::AssociatedNamespaceSet::iterator + it = AssociatedNamespaces.begin(), + end = AssociatedNamespaces.end(); it != end; ++it) + if (Std->Encloses(*it)) + StdNamespaces.push_back(*it); + for (unsigned I = 0; I != StdNamespaces.size(); ++I) + AssociatedNamespaces.erase(StdNamespaces[I]); + } + + SemaRef.Diag(R.getNameLoc(), diag::err_not_found_by_two_phase_lookup) + << R.getLookupName(); + if (AssociatedNamespaces.empty()) { + SemaRef.Diag(Best->Function->getLocation(), + diag::note_not_found_by_two_phase_lookup) + << R.getLookupName() << 0; + } else if (AssociatedNamespaces.size() == 1) { + SemaRef.Diag(Best->Function->getLocation(), + diag::note_not_found_by_two_phase_lookup) + << R.getLookupName() << 1 << *AssociatedNamespaces.begin(); + } else { + // FIXME: It would be useful to list the associated namespaces here, + // but the diagnostics infrastructure doesn't provide a way to produce + // a localized representation of a list of items. + SemaRef.Diag(Best->Function->getLocation(), + diag::note_not_found_by_two_phase_lookup) + << R.getLookupName() << 2; + } + + // Try to recover by calling this function. + return true; + } + + R.clear(); + } + + return false; +} + +/// Attempt to recover from ill-formed use of a non-dependent operator in a +/// template, where the non-dependent operator was declared after the template +/// was defined. +/// +/// Returns true if a viable candidate was found and a diagnostic was issued. +static bool +DiagnoseTwoPhaseOperatorLookup(Sema &SemaRef, OverloadedOperatorKind Op, + SourceLocation OpLoc, + Expr **Args, unsigned NumArgs) { + DeclarationName OpName = + SemaRef.Context.DeclarationNames.getCXXOperatorName(Op); + LookupResult R(SemaRef, OpName, OpLoc, Sema::LookupOperatorName); + return DiagnoseTwoPhaseLookup(SemaRef, OpLoc, CXXScopeSpec(), R, + /*ExplicitTemplateArgs=*/0, Args, NumArgs); +} + /// Attempts to recover from a call where no functions were found. /// /// Returns true if new candidates were found. @@ -7834,13 +7939,14 @@ BuildRecoveryCallExpr(Sema &SemaRef, Scope *S, Expr *Fn, UnresolvedLookupExpr *ULE, SourceLocation LParenLoc, Expr **Args, unsigned NumArgs, - SourceLocation RParenLoc) { + SourceLocation RParenLoc, + bool EmptyLookup) { CXXScopeSpec SS; SS.Adopt(ULE->getQualifierLoc()); TemplateArgumentListInfo TABuffer; - const TemplateArgumentListInfo *ExplicitTemplateArgs = 0; + TemplateArgumentListInfo *ExplicitTemplateArgs = 0; if (ULE->hasExplicitTemplateArgs()) { ULE->copyTemplateArgumentsInto(TABuffer); ExplicitTemplateArgs = &TABuffer; @@ -7848,7 +7954,10 @@ BuildRecoveryCallExpr(Sema &SemaRef, Scope *S, Expr *Fn, LookupResult R(SemaRef, ULE->getName(), ULE->getNameLoc(), Sema::LookupOrdinaryName); - if (SemaRef.DiagnoseEmptyLookup(S, SS, R, Sema::CTC_Expression)) + if (!DiagnoseTwoPhaseLookup(SemaRef, Fn->getExprLoc(), SS, R, + ExplicitTemplateArgs, Args, NumArgs) && + (!EmptyLookup || + SemaRef.DiagnoseEmptyLookup(S, SS, R, Sema::CTC_Expression))) return ExprError(); assert(!R.empty() && "lookup results empty despite recovery"); @@ -7868,7 +7977,7 @@ BuildRecoveryCallExpr(Sema &SemaRef, Scope *S, Expr *Fn, return ExprError(); // This shouldn't cause an infinite loop because we're giving it - // an expression with non-empty lookup results, which should never + // an expression with viable lookup results, which should never // end up here. return SemaRef.ActOnCallExpr(/*Scope*/ 0, NewFn.take(), LParenLoc, MultiExprArg(Args, NumArgs), RParenLoc); @@ -7914,11 +8023,11 @@ Sema::BuildOverloadedCallExpr(Scope *S, Expr *Fn, UnresolvedLookupExpr *ULE, AddOverloadedCallCandidates(ULE, Args, NumArgs, CandidateSet); // If we found nothing, try to recover. - // AddRecoveryCallCandidates diagnoses the error itself, so we just - // bailout out if it fails. + // BuildRecoveryCallExpr diagnoses the error itself, so we just bail + // out if it fails. if (CandidateSet.empty()) return BuildRecoveryCallExpr(*this, S, Fn, ULE, LParenLoc, Args, NumArgs, - RParenLoc); + RParenLoc, /*EmptyLookup=*/true); OverloadCandidateSet::iterator Best; switch (CandidateSet.BestViableFunction(*this, Fn->getLocStart(), Best)) { @@ -7933,12 +8042,21 @@ Sema::BuildOverloadedCallExpr(Scope *S, Expr *Fn, UnresolvedLookupExpr *ULE, ExecConfig); } - case OR_No_Viable_Function: + case OR_No_Viable_Function: { + // Try to recover by looking for viable functions which the user might + // have meant to call. + ExprResult Recovery = BuildRecoveryCallExpr(*this, S, Fn, ULE, LParenLoc, + Args, NumArgs, RParenLoc, + /*EmptyLookup=*/false); + if (!Recovery.isInvalid()) + return Recovery; + Diag(Fn->getSourceRange().getBegin(), diag::err_ovl_no_viable_function_in_call) << ULE->getName() << Fn->getSourceRange(); CandidateSet.NoteCandidates(*this, OCD_AllCandidates, Args, NumArgs); break; + } case OR_Ambiguous: Diag(Fn->getSourceRange().getBegin(), diag::err_ovl_ambiguous_call) @@ -8127,6 +8245,13 @@ Sema::CreateOverloadedUnaryOp(SourceLocation OpLoc, unsigned OpcIn, } case OR_No_Viable_Function: + // This is an erroneous use of an operator which can be overloaded by + // a non-member function. Check for non-member operators which were + // defined too late to be candidates. + if (DiagnoseTwoPhaseOperatorLookup(*this, Op, OpLoc, Args, NumArgs)) + // FIXME: Recover by calling the found function. + return ExprError(); + // No viable function; fall through to handling this as a // built-in operator, which will produce an error message for us. break; @@ -8408,6 +8533,13 @@ Sema::CreateOverloadedBinOp(SourceLocation OpLoc, << BinaryOperator::getOpcodeStr(Opc) << Args[0]->getSourceRange() << Args[1]->getSourceRange(); } else { + // This is an erroneous use of an operator which can be overloaded by + // a non-member function. Check for non-member operators which were + // defined too late to be candidates. + if (DiagnoseTwoPhaseOperatorLookup(*this, Op, OpLoc, Args, 2)) + // FIXME: Recover by calling the found function. + return ExprError(); + // No viable function; try to create a built-in operation, which will // produce an error. Then, show the non-viable candidates. Result = CreateBuiltinBinOp(OpLoc, Opc, Args[0], Args[1]); diff --git a/test/SemaTemplate/dependent-names.cpp b/test/SemaTemplate/dependent-names.cpp index 5776ddca2c..2a50df5b7f 100644 --- a/test/SemaTemplate/dependent-names.cpp +++ b/test/SemaTemplate/dependent-names.cpp @@ -1,4 +1,4 @@ -// RUN: %clang_cc1 -fsyntax-only -verify %s +// RUN: %clang_cc1 -fsyntax-only -verify -std=c++0x %s typedef double A; template<typename T> class B { @@ -129,3 +129,136 @@ namespace PR8966 { template <class T> const char* MyClass<T>::array [MyClass<T>::N] = { "A", "B", "C" }; } + +namespace std { + inline namespace v1 { + template<typename T> struct basic_ostream; + } + namespace inner { + template<typename T> struct vector {}; + } + using inner::vector; + template<typename T, typename U> struct pair {}; + typedef basic_ostream<char> ostream; + extern ostream cout; + std::ostream &operator<<(std::ostream &out, const char *); +} + +namespace PR10053 { + template<typename T> struct A { + T t; + A() { + f(t); // expected-error {{call to function 'f' that is neither visible in the template definition nor found by argument dependent lookup}} + } + }; + + void f(int&); // expected-note {{'f' should be declared prior to the call site}} + + A<int> a; // expected-note {{in instantiation of member function}} + + + namespace N { + namespace M { + template<typename T> int g(T t) { + f(t); // expected-error {{call to function 'f' that is neither visible in the template definition nor found by argument dependent lookup}} + }; + } + + void f(char&); // expected-note {{'f' should be declared prior to the call site}} + } + + void f(char&); + + int k = N::M::g<char>(0);; // expected-note {{in instantiation of function}} + + + namespace O { + void f(char&); // expected-note {{candidate function not viable}} + + template<typename T> struct C { + static const int n = f(T()); // expected-error {{no matching function}} + }; + } + + int f(double); // no note, shadowed by O::f + O::C<double> c; // expected-note {{requested here}} + + + // Example from www/compatibility.html + namespace my_file { + template <typename T> T Squared(T x) { + return Multiply(x, x); // expected-error {{neither visible in the template definition nor found by argument dependent lookup}} + } + + int Multiply(int x, int y) { // expected-note {{should be declared prior to the call site}} + return x * y; + } + + int main() { + Squared(5); // expected-note {{here}} + } + } + + // Example from www/compatibility.html + namespace my_file2 { + template<typename T> + void Dump(const T& value) { + std::cout << value << "\n"; // expected-error {{neither visible in the template definition nor found by argument dependent lookup}} + } + + namespace ns { + struct Data {}; + } + + std::ostream& operator<<(std::ostream& out, ns::Data data) { // expected-note {{should be declared prior to the call site or in namespace 'PR10053::my_file2::ns'}} + return out << "Some data"; + } + + void Use() { + Dump(ns::Data()); // expected-note {{here}} + } + } + + namespace my_file2_a { + template<typename T> + void Dump(const T &value) { + print(std::cout, value); // expected-error 4{{neither visible in the template definition nor found by argument dependent lookup}} + } + + namespace ns { + struct Data {}; + } + namespace ns2 { + struct Data {}; + } + + std::ostream &print(std::ostream &out, int); // expected-note-re {{should be declared prior to the call site$}} + std::ostream &print(std::ostream &out, ns::Data); // expected-note {{should be declared prior to the call site or in namespace 'PR10053::my_file2_a::ns'}} + std::ostream &print(std::ostream &out, std::vector<ns2::Data>); // expected-note {{should be declared prior to the call site or in namespace 'PR10053::my_file2_a::ns2'}} + std::ostream &print(std::ostream &out, std::pair<ns::Data, ns2::Data>); // expected-note {{should be declared prior to the call site or in an associated namespace of one of its arguments}} + + void Use() { + Dump(0); // expected-note {{requested here}} + Dump(ns::Data()); // expected-note {{requested here}} + Dump(std::vector<ns2::Data>()); // expected-note {{requested here}} + Dump(std::pair<ns::Data, ns2::Data>()); // expected-note {{requested here}} + } + } + + namespace unary { + template<typename T> + T Negate(const T& value) { + return !value; // expected-error {{call to function 'operator!' that is neither visible in the template definition nor found by argument dependent lookup}} + } + + namespace ns { + struct Data {}; + } + + ns::Data operator!(ns::Data); // expected-note {{should be declared prior to the call site or in namespace 'PR10053::unary::ns'}} + + void Use() { + Negate(ns::Data()); // expected-note {{requested here}} + } + } +} diff --git a/test/SemaTemplate/instantiate-call.cpp b/test/SemaTemplate/instantiate-call.cpp index ad06be33aa..a0e48c8dec 100644 --- a/test/SemaTemplate/instantiate-call.cpp +++ b/test/SemaTemplate/instantiate-call.cpp @@ -24,7 +24,8 @@ namespace N3 { template<typename T, typename Result> struct call_f0 { void test_f0(T t) { - Result &result = f0(t); // expected-error 2{{undeclared identifier}} + Result &result = f0(t); // expected-error {{undeclared identifier}} \ + expected-error {{neither visible in the template definition nor found by argument dependent lookup}} } }; } @@ -32,7 +33,7 @@ namespace N3 { template struct N3::call_f0<int, char&>; // expected-note{{instantiation}} template struct N3::call_f0<N1::X0, int&>; -short& f0(char); +short& f0(char); // expected-note {{should be declared prior to the call site}} namespace N4 { template<typename T, typename Result> struct call_f0 { diff --git a/www/compatibility.html b/www/compatibility.html index aa6a39dda1..b68a7663d4 100644 --- a/www/compatibility.html +++ b/www/compatibility.html @@ -459,13 +459,15 @@ int main() { <p>Clang complains: -<pre> <b>my_file.cpp:2:10: <span class="error">error:</span> use of undeclared identifier 'Multiply'</b> +<pre> <b>my_file.cpp:2:10: <span class="error">error:</span> call to function 'Multiply' that is neither visible in the template definition nor found by argument dependent lookup</b> return Multiply(x, x); <span class="caret"> ^</span> - <b>my_file.cpp:10:3: <span class="note">note:</span> in instantiation of function template specialization 'Squared<int>' requested here</b> Squared(5); <span class="caret"> ^</span> + <b>my_file.cpp:5:5: <span class="note">note:</span> 'Multiply' should be declared prior to the call site</b> + int Multiply(int x, int y) { + <span class="caret"> ^</span> </pre> <p>The C++ standard says that unqualified names like <q>Multiply</q> @@ -516,15 +518,17 @@ void Use() { Dump(ns::Data()); }</pre> -<p>Again, Clang complains about not finding a matching function:</p> +<p>Again, Clang complains:</p> -<pre> -<b>my_file.cpp:5:13: <span class="error">error:</span> invalid operands to binary expression ('ostream' (aka 'basic_ostream<char>') and 'ns::Data const')</b> - std::cout << value << "\n"; - <span class="caret">~~~~~~~~~ ^ ~~~~~</span> -<b>my_file.cpp:17:3: <span class="note">note:</span> in instantiation of function template specialization 'Dump<ns::Data>' requested here</b> - Dump(ns::Data()); - <span class="caret">^</span> +<pre> <b>my_file2.cpp:5:13: <span class="error">error:</span> call to function 'operator<<' that is neither visible in the template definition nor found by argument dependent lookup</b> + std::cout << value << "\n"; + <span class="caret"> ^</span> + <b>my_file2.cpp:17:3: <span class="error">note:</span> in instantiation of function template specialization 'Dump<ns::Data>' requested here</b> + Dump(ns::Data()); + <span class="caret"> ^</span> + <b>my_file2.cpp:12:15: <span class="error">note:</span> 'operator<<' should be declared prior to the call site or in namespace 'ns'</b> + std::ostream& operator<<(std::ostream& out, ns::Data data) { + <span class="caret"> ^</span> </pre> <p>Just like before, unqualified lookup didn't find any declarations |