Improve diagnostics for ambiguous name lookup results

llvm-svn: 62287

clang/include/clang/Basic/DiagnosticKinds.def

@@ -1542,9 +1542,11 @@ DIAG(err_ambiguous_derived_to_base_conv, ERROR,
 
 // C++ member name lookup
 DIAG(err_ambiguous_member_multiple_subobjects, ERROR,
-     "non-static member %0 found in multiple base-class subobjects of type %1")
+     "non-static member %0 found in multiple base-class subobjects of type %1:%2")
 DIAG(err_ambiguous_member_multiple_subobject_types, ERROR,
      "member %0 found in multiple base classes of different types")
+DIAG(note_ambiguous_member_found, NOTE,
+     "member found by ambiguous name lookup")
 
 // C++ operator overloading
 DIAG(err_operator_overload_needs_class_or_enum, ERROR,

clang/lib/Sema/Sema.h

@@ -25,6 +25,7 @@
 #include "llvm/ADT/DenseSet.h"
 #include "llvm/ADT/SmallPtrSet.h"
 #include "llvm/ADT/OwningPtr.h"
+#include <string>
 #include <vector>
 
 namespace llvm {
@@ -1334,6 +1335,7 @@ public:
                      BasePaths &Paths);
   bool CheckDerivedToBaseConversion(QualType Derived, QualType Base,
                                     SourceLocation Loc, SourceRange Range);
+  std::string getAmbiguousPathsDisplayString(BasePaths &Paths);
 
   //===--------------------------------------------------------------------===//
   // C++ Overloaded Operators [C++ 13.5]

clang/lib/Sema/SemaInherit.cpp

@@ -49,6 +49,7 @@ void BasePaths::clear() {
 /// @brief Swaps the contents of this BasePaths structure with the
 /// contents of Other.
 void BasePaths::swap(BasePaths &Other) {
+  std::swap(Origin, Other.Origin);
   Paths.swap(Other.Paths);
   ClassSubobjects.swap(Other.ClassSubobjects);
   std::swap(FindAmbiguities, Other.FindAmbiguities);
@@ -86,6 +87,7 @@ bool Sema::IsDerivedFrom(QualType Derived, QualType Base, BasePaths &Paths) {
   if (Derived == Base)
     return false;
 
+  Paths.setOrigin(Derived);
   return LookupInBases(cast<CXXRecordType>(Derived->getAsRecordType())->getDecl(),
                        MemberLookupCriteria(Base), Paths);
 }
@@ -240,6 +242,26 @@ Sema::CheckDerivedToBaseConversion(QualType Derived, QualType Base,
   // D -> B -> A, that will be used to illustrate the ambiguous
   // conversions in the diagnostic. We only print one of the paths
   // to each base class subobject.
+  std::string PathDisplayStr = getAmbiguousPathsDisplayString(Paths);
+  
+  Diag(Loc, diag::err_ambiguous_derived_to_base_conv)
+    << Derived << Base << PathDisplayStr << Range;
+  return true;
+}
+
+/// @brief Builds a string representing ambiguous paths from a
+/// specific derived class to different subobjects of the same base
+/// class.
+///
+/// This function builds a string that can be used in error messages
+/// to show the different paths that one can take through the
+/// inheritance hierarchy to go from the derived class to different
+/// subobjects of a base class. The result looks something like this:
+/// @code
+/// struct D -> struct B -> struct A
+/// struct D -> struct C -> struct A
+/// @endcode
+std::string Sema::getAmbiguousPathsDisplayString(BasePaths &Paths) {
   std::string PathDisplayStr;
   std::set<unsigned> DisplayedPaths;
   for (BasePaths::paths_iterator Path = Paths.begin(); 
@@ -248,14 +270,12 @@ Sema::CheckDerivedToBaseConversion(QualType Derived, QualType Base,
       // We haven't displayed a path to this particular base
       // class subobject yet.
       PathDisplayStr += "\n    ";
-      PathDisplayStr += Derived.getAsString();
+      PathDisplayStr += Paths.getOrigin().getAsString();
       for (BasePath::const_iterator Element = Path->begin(); 
            Element != Path->end(); ++Element)
         PathDisplayStr += " -> " + Element->Base->getType().getAsString(); 
     }
   }
-  
-  Diag(Loc, diag::err_ambiguous_derived_to_base_conv)
-    << Derived << Base << PathDisplayStr << Range;
-  return true;
+
+  return PathDisplayStr;
 }

clang/lib/Sema/SemaInherit.h

@@ -92,6 +92,9 @@ namespace clang {
   /// refer to the same base class subobject of type A (the virtual
   /// one), there is no ambiguity.
   class BasePaths {
+    /// Origin - The type from which this search originated.
+    QualType Origin;
+
     /// Paths - The actual set of paths that can be taken from the
     /// derived class to the same base class.
     std::list<BasePath> Paths;
@@ -168,6 +171,11 @@ namespace clang {
       return DetectedVirtual;
     }
 
+    /// @brief Retrieve the type from which this base-paths search
+    /// began
+    QualType getOrigin() const { return Origin; }
+    void setOrigin(QualType Type) { Origin = Type; }
+
     void clear();
 
     void swap(BasePaths &Other);

clang/lib/Sema/SemaLookup.cpp

@@ -20,6 +20,7 @@
 #include "clang/Parse/DeclSpec.h"
 #include "clang/Basic/LangOptions.h"
 #include "llvm/ADT/STLExtras.h"
+#include <set>
 
 using namespace clang;
 
@@ -476,6 +477,7 @@ Sema::LookupQualifiedName(DeclContext *LookupCtx, DeclarationName Name,
 
   // Perform lookup into our base classes.
   BasePaths Paths;
+  Paths.setOrigin(Context.getTypeDeclType(cast<RecordDecl>(LookupCtx)));
 
   // Look for this member in our base classes
   if (!LookupInBases(cast<CXXRecordDecl>(LookupCtx), 
@@ -611,11 +613,19 @@ bool Sema::DiagnoseAmbiguousLookup(LookupResult &Result, DeclarationName Name,
                                    SourceRange LookupRange) {
   assert(Result.isAmbiguous() && "Lookup result must be ambiguous");
 
+  BasePaths *Paths = Result.getBasePaths();
   if (Result.getKind() == LookupResult::AmbiguousBaseSubobjects) {
-    BasePaths *Paths = Result.getBasePaths();
     QualType SubobjectType = Paths->front().back().Base->getType();
-    return Diag(NameLoc, diag::err_ambiguous_member_multiple_subobjects)
-      << Name << SubobjectType << LookupRange;
+    Diag(NameLoc, diag::err_ambiguous_member_multiple_subobjects)
+      << Name << SubobjectType << getAmbiguousPathsDisplayString(*Paths)
+      << LookupRange;
+
+    DeclContext::lookup_iterator Found = Paths->front().Decls.first;
+    while (isa<CXXMethodDecl>(*Found) && cast<CXXMethodDecl>(*Found)->isStatic())
+      ++Found;
+
+    Diag((*Found)->getLocation(), diag::note_ambiguous_member_found);
+    return true;
   } 
 
   assert(Result.getKind() == LookupResult::AmbiguousBaseSubobjectTypes &&
@@ -624,7 +634,13 @@ bool Sema::DiagnoseAmbiguousLookup(LookupResult &Result, DeclarationName Name,
   Diag(NameLoc, diag::err_ambiguous_member_multiple_subobject_types)
     << Name << LookupRange;
 
-  // FIXME: point out the members we found using notes.
+  std::set<ScopedDecl *> DeclsPrinted;
+  for (BasePaths::paths_iterator Path = Paths->begin(), PathEnd = Paths->end();
+       Path != PathEnd; ++Path) {
+    ScopedDecl *D = *Path->Decls.first;
+    if (DeclsPrinted.insert(D).second)
+      Diag(D->getLocation(), diag::note_ambiguous_member_found);
+  }
+
   return true;
 }
-