[Clang][Sema] Fix regression due to missing ambiguity check before attempting access check.

[shafik]

Previously when fixing ambiguous lookup diagnostics in cc1b666 The change refactored LookupResult to split out diagnosing access and ambiguous lookups. The call to getSema().CheckLookupAccess(...) should have guarded by a check for isAmbiguous(). This change adds that guard.

Fixes: #80435

[llvmbot]

@llvm/pr-subscribers-clang

Author: Shafik Yaghmour (shafik)

Changes

Previously when fixing ambiguous lookup diagnostics in cc1b666 The change refactored LookupResult to split out diagnosing access and ambiguous lookups. The call to getSema().CheckLookupAccess(...) should have guarded by a check for isAmbiguous(). This change adds that guard.

Fixes: #80435

---

Full diff: https://github.com/llvm/llvm-project/pull/80730.diff

3 Files Affected:

• (modified) clang/docs/ReleaseNotes.rst (+2)
• (modified) clang/include/clang/Sema/Lookup.h (+1-1)
• (modified) clang/test/CXX/class.derived/class.member.lookup/p11.cpp (+22)

diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 3596109bf044f..6c00bbe8a0eda 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -201,6 +201,8 @@ Bug Fixes to C++ Support
   parameter where we did an incorrect specialization of the initialization of
   the default parameter.
   Fixes (`#68490 <https://github.com/llvm/llvm-project/issues/68490>`_)
+- Fix regression due missing ambiguity check before checking lookup access.
+  Fixes (`80435 <https://github.com/llvm/llvm-project/issues/80435>`_)
 
 Bug Fixes to AST Handling
 ^^^^^^^^^^^^^^^^^^^^^^^^^
diff --git a/clang/include/clang/Sema/Lookup.h b/clang/include/clang/Sema/Lookup.h
index 9c93bf1e6fb42..d325e394c1b8d 100644
--- a/clang/include/clang/Sema/Lookup.h
+++ b/clang/include/clang/Sema/Lookup.h
@@ -754,7 +754,7 @@ class LookupResult {
 
 private:
   void diagnoseAccess() {
-    if (isClassLookup() && getSema().getLangOpts().AccessControl)
+    if (!isAmbiguous() && isClassLookup() && getSema().getLangOpts().AccessControl)
       getSema().CheckLookupAccess(*this);
   }
 
diff --git a/clang/test/CXX/class.derived/class.member.lookup/p11.cpp b/clang/test/CXX/class.derived/class.member.lookup/p11.cpp
index e0899b227e69b..6843406d98ea2 100644
--- a/clang/test/CXX/class.derived/class.member.lookup/p11.cpp
+++ b/clang/test/CXX/class.derived/class.member.lookup/p11.cpp
@@ -23,3 +23,25 @@ struct D: I1, I2, B2 {
     int D::* mpD = &D::i; // expected-error {{non-static member 'i' found in multiple base-class subobjects of type 'B1'}}
   }
 };
+
+namespace GH80435 {
+struct A {
+  void *data; // expected-note {{member found by ambiguous name lookup}}
+};
+
+class B {
+  void *data; // expected-note {{member found by ambiguous name lookup}}
+};
+
+struct C : A, B {};
+
+decltype(C().data) x; // expected-error {{member 'data' found in multiple base classes of different types}}
+
+struct D { // expected-note {{candidate constructor (the implicit copy constructor) not viable: no known conversion from 'C' to 'const D' for 1st argument}} 
+           // expected-note@-1{{candidate constructor (the implicit move constructor) not viable: no known conversion from 'C' to 'D' for 1st argument}}
+  template <typename Container, decltype(Container().data) = 0 > 
+  D(Container); // expected-note {{candidate template ignored: substitution failure [with Container = C]: member 'data' found in multiple base classes of different types}}
+};
+
+D y(C{}); // expected-error {{no matching constructor for initialization of 'D'}}
+}

[erichkeane]

This'll need a cherry pick to 18 as it is a break against 17, right? Otherwise LGTM.

[cor3ntin]

LGTM

[github-actions]

✅ With the latest revision this PR passed the C/C++ code formatter.