From: Richard Smith Date: Sun, 5 Jun 2011 22:42:48 +0000 (+0000) Subject: Fix PR10053: Improve diagnostics and error recovery for code which some compilers... X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=f50e88a793dd5bc7073c717fec78912e3234e95a;p=clang 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 --- 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; 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(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 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 class B { @@ -129,3 +129,136 @@ namespace PR8966 { template const char* MyClass::array [MyClass::N] = { "A", "B", "C" }; } + +namespace std { + inline namespace v1 { + template struct basic_ostream; + } + namespace inner { + template struct vector {}; + } + using inner::vector; + template struct pair {}; + typedef basic_ostream ostream; + extern ostream cout; + std::ostream &operator<<(std::ostream &out, const char *); +} + +namespace PR10053 { + template 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 a; // expected-note {{in instantiation of member function}} + + + namespace N { + namespace M { + template 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(0);; // expected-note {{in instantiation of function}} + + + namespace O { + void f(char&); // expected-note {{candidate function not viable}} + + template struct C { + static const int n = f(T()); // expected-error {{no matching function}} + }; + } + + int f(double); // no note, shadowed by O::f + O::C c; // expected-note {{requested here}} + + + // Example from www/compatibility.html + namespace my_file { + template 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 + 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 + 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); // 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); // 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()); // expected-note {{requested here}} + Dump(std::pair()); // expected-note {{requested here}} + } + } + + namespace unary { + template + 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 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; // expected-note{{instantiation}} template struct N3::call_f0; -short& f0(char); +short& f0(char); // expected-note {{should be declared prior to the call site}} namespace N4 { template 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() {

Clang complains: -

  my_file.cpp:2:10: error: use of undeclared identifier 'Multiply'
+
  my_file.cpp:2:10: error: call to function 'Multiply' that is neither visible in the template definition nor found by argument dependent lookup
     return Multiply(x, x);
            ^
-
   my_file.cpp:10:3: note: in instantiation of function template specialization 'Squared<int>' requested here
     Squared(5);
     ^
+  my_file.cpp:5:5: note: 'Multiply' should be declared prior to the call site
+  int Multiply(int x, int y) {
+      ^
 

 
 
The C++ standard says that unqualified names like Multiply
@@ -516,15 +518,17 @@ void Use() {
   Dump(ns::Data());
 }
-

Again, Clang complains about not finding a matching function:

+

Again, Clang complains:

-
-my_file.cpp:5:13: error: invalid operands to binary expression ('ostream' (aka 'basic_ostream<char>') and 'ns::Data const')
-  std::cout << value << "\n";
-  ~~~~~~~~~ ^  ~~~~~
-my_file.cpp:17:3: note: in instantiation of function template specialization 'Dump<ns::Data>' requested here
-  Dump(ns::Data());
-  ^
+
  my_file2.cpp:5:13: error: call to function 'operator<<' that is neither visible in the template definition nor found by argument dependent lookup
+    std::cout << value << "\n";
+              ^
+  my_file2.cpp:17:3: note: in instantiation of function template specialization 'Dump<ns::Data>' requested here
+    Dump(ns::Data());
+    ^
+  my_file2.cpp:12:15: note: 'operator<<' should be declared prior to the call site or in namespace 'ns'
+  std::ostream& operator<<(std::ostream& out, ns::Data data) {
+                ^
 

 
 
Just like before, unqualified lookup didn't find any declarations