[PATCH] D155387: [Clang] Fix member lookup so that we don't ignore ambiguous lookups in some cases

[Richard Smith - zygoloid via Phabricator via cfe-commits]

rsmith added inline comments.


================
Comment at: clang/include/clang/Sema/Lookup.h:228-229
     Other.Paths = nullptr;
-    Other.Diagnose = false;
+    Other.DiagnoseAccess = false;
+    Other.DiagnoseAmbiguous = false;
     return *this;
----------------
cor3ntin wrote:
> Does anything break if you remove these two lines? they don't appear useful
I think these make sense: if we move a lookup result into this one, then the other lookup result shouldn't issue diagnostics any more. (Otherwise we could see the same diagnostics twice.)


================
Comment at: clang/lib/Sema/SemaOverload.cpp:14930
   LookupQualifiedName(R, Record->getDecl());
-  R.suppressDiagnostics();
+  R.suppressAccessDiagnostics();
 
----------------
shafik wrote:
> I was a bit conservative where I changed this but maybe we should do this everywhere.
Most of the other calls in this file all look wrong. `DiagnoseTwoPhaseLookup` and `BuildRecoveryCallExpr` are doing error recovery and I think it's appropriate for them to suppress all diagnostics, but the rest are doing standard-mandated searches, and so should diagnose lookup ambiguity.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D155387/new/

https://reviews.llvm.org/D155387