[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 marked 4 inline comments as done.
shafik added inline comments.


================
Comment at: clang/lib/Sema/SemaOverload.cpp:15191
   OverloadCandidateSet::iterator Best;
   switch (CandidateSet.BestViableFunction(*this, OpLoc, Best)) {
   case OR_Success:
----------------
rsmith wrote:
> shafik wrote:
> > @rsmith if `R.isAmbiguous()` should we even check the overload candidates? 
> > 
> > Right now in `clang/test/SemaCXX/arrow-operator.cpp` we are getting both an ambiguous name lookup and overload diagnostic. 
> Hm, I think perhaps the ideal thing to do is: if `BestViableFunction` returns `OR_Ambiguous`, the lookup was ambiguous, and we found viable functions in multiple different base classes, then don't produce any further diagnostics. That way, we still get good error recovery in the case where the overloading is unambiguous despite the lookup being ambiguous, or when the overloading is ambiguous for some reason unrelated to the ambiguous lookup.
> 
> (We'd want to do this in all the overloaded operator lookups in this file, not just for `operator->`.)
I only found one other location that matched this pattern if you meant a larger change let me know. 


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

https://reviews.llvm.org/D155387