Skip to content

Conversation

llvmbot
Copy link
Member

@llvmbot llvmbot commented Feb 6, 2024

resolves #80887

@llvmbot llvmbot added this to the LLVM 18.X Release milestone Feb 6, 2024
@llvmbot
Copy link
Member Author

llvmbot commented Feb 6, 2024

@shafik What do you think about merging this PR to the release branch?

@llvmbot llvmbot requested a review from shafik February 6, 2024 20:00
@llvmbot llvmbot added clang Clang issues not falling into any other category clang:frontend Language frontend issues, e.g. anything involving "Sema" labels Feb 6, 2024
@llvmbot
Copy link
Member Author

llvmbot commented Feb 6, 2024

@llvm/pr-subscribers-clang

Author: None (llvmbot)

Changes

resolves llvm/llvm-project#80887


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

2 Files Affected:

  • (modified) clang/include/clang/Sema/Lookup.h (+2-1)
  • (modified) clang/test/CXX/class.derived/class.member.lookup/p11.cpp (+22)
diff --git a/clang/include/clang/Sema/Lookup.h b/clang/include/clang/Sema/Lookup.h
index 9c93bf1e6fb428..2f2f2607a937fe 100644
--- a/clang/include/clang/Sema/Lookup.h
+++ b/clang/include/clang/Sema/Lookup.h
@@ -754,7 +754,8 @@ 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 e0899b227e69bd..a42febaca3f041 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'}}
+}

Copy link
Contributor

@cor3ntin cor3ntin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This fixes a regression introduced in 18 and the fix is small,

Ship it :)

…tempting access check. (llvm#80730)

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: llvm#80435
(cherry picked from commit a7bc9cb)
@tstellar tstellar merged commit 2427c85 into llvm:release/18.x Feb 7, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clang:frontend Language frontend issues, e.g. anything involving "Sema" clang Clang issues not falling into any other category
Projects
Development

Successfully merging this pull request may close these issues.

4 participants