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
2011-06-05 22:42:48 +00:00 · 2011-06-05 22:42:48 +00:00 · f50e88a793
--- 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>;
--- 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]);
--- 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}}
+    }
+  }
+}
--- 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 {
--- 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&lt;int&gt;' 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&lt;char&gt;') and 'ns::Data const')</b>
-  std::cout &lt;&lt; value &lt;&lt; "\n";
-  <span class="caret">~~~~~~~~~ ^  ~~~~~</span>
-<b>my_file.cpp:17:3: <span class="note">note:</span> in instantiation of function template specialization 'Dump&lt;ns::Data&gt;' 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&lt;&lt;' that is neither visible in the template definition nor found by argument dependent lookup</b>
+    std::cout &lt;&lt; value &lt;&lt; "\n";
+  <span class="caret">            ^</span>
+  <b>my_file2.cpp:17:3: <span class="error">note:</span> in instantiation of function template specialization 'Dump&lt;ns::Data&gt;' requested here</b>
+    Dump(ns::Data());
+  <span class="caret">  ^</span>
+  <b>my_file2.cpp:12:15: <span class="error">note:</span> 'operator&lt;&lt;' should be declared prior to the call site or in namespace 'ns'</b>
+  std::ostream&amp; operator&lt;&lt;(std::ostream&amp; out, ns::Data data) {
+  <span class="caret">              ^</span>
 </pre>

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