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

[Shafik Yaghmour via Phabricator via cfe-commits]

shafik added inline comments.


================
Comment at: clang/lib/Sema/SemaOverload.cpp:14930
   LookupQualifiedName(R, Record->getDecl());
-  R.suppressDiagnostics();
+  R.suppressAccessDiagnostics();
 
----------------
shafik wrote:
> rsmith wrote:
> > 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.
> So it looks like `BuildOverloadedArrowExpr` catches the ambiguous lookup when calling `CandidateSet.BestViableFunction(...)` while `BuildCallToObjectOfClassType(...)` does not. So if I change the `suppressDiagnostics()` call in `BuildOverloadedArrowExpr` we get double diagnostic. I am not seeing why we have the inconsistency here.  
Correction, there are not duplicate diagnostics, one is for ambiguous overload and the other is for ambiguous name lookup. Maybe we should suppress the ambiguous overload if we have an ambiguous name lookup though.


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

https://reviews.llvm.org/D155387