Skip to content

Revert "[Clang] Fix name lookup for dependent bases (#114978)" #117727

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Nov 26, 2024

Conversation

asi-sc
Copy link
Contributor

@asi-sc asi-sc commented Nov 26, 2024

@asi-sc asi-sc requested a review from Endilll as a code owner November 26, 2024 15:40
@llvmbot llvmbot added clang Clang issues not falling into any other category clang:frontend Language frontend issues, e.g. anything involving "Sema" labels Nov 26, 2024
@llvmbot
Copy link
Member

llvmbot commented Nov 26, 2024

@llvm/pr-subscribers-clang

Author: Anton Sidorenko (asi-sc)

Changes

This reverts commit 4866447 as requested by the commit author.

Buildbots fail:


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

4 Files Affected:

  • (modified) clang/docs/ReleaseNotes.rst (-3)
  • (modified) clang/lib/AST/CXXInheritance.cpp (+6-12)
  • (modified) clang/test/CXX/drs/cwg5xx.cpp (+2-46)
  • (modified) clang/www/cxx_dr_status.html (+1-1)
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index c4086a5bcbf368..6c40e48e2f49b3 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -279,9 +279,6 @@ Resolutions to C++ Defect Reports
   by default.
   (`CWG2521: User-defined literals and reserved identifiers <https://cplusplus.github.io/CWG/issues/2521.html>`_).
 
-- Fix name lookup for a dependent base class that is the current instantiation.  
-  (`CWG591: When a dependent base class is the current instantiation <https://cplusplus.github.io/CWG/issues/591.html>`_).
-
 C Language Changes
 ------------------
 
diff --git a/clang/lib/AST/CXXInheritance.cpp b/clang/lib/AST/CXXInheritance.cpp
index 10b8d524ff8978..aefc06e9197cfb 100644
--- a/clang/lib/AST/CXXInheritance.cpp
+++ b/clang/lib/AST/CXXInheritance.cpp
@@ -134,7 +134,7 @@ bool CXXRecordDecl::forallBases(ForallBasesCallback BaseMatches) const {
         return false;
 
       CXXRecordDecl *Base =
-          cast_if_present<CXXRecordDecl>(Ty->getDecl()->getDefinition());
+            cast_or_null<CXXRecordDecl>(Ty->getDecl()->getDefinition());
       if (!Base ||
           (Base->isDependentContext() &&
            !Base->isCurrentInstantiation(Record))) {
@@ -169,21 +169,13 @@ bool CXXBasePaths::lookupInBases(ASTContext &Context,
     QualType BaseType =
         Context.getCanonicalType(BaseSpec.getType()).getUnqualifiedType();
 
-    bool isCurrentInstantiation = isa<InjectedClassNameType>(BaseType);
-    if (!isCurrentInstantiation) {
-      if (auto *BaseRecord = cast_if_present<CXXRecordDecl>(
-              BaseSpec.getType()->getAsRecordDecl()))
-        isCurrentInstantiation = BaseRecord->isDependentContext() &&
-                                 BaseRecord->isCurrentInstantiation(Record);
-    }
     // C++ [temp.dep]p3:
     //   In the definition of a class template or a member of a class template,
     //   if a base class of the class template depends on a template-parameter,
     //   the base class scope is not examined during unqualified name lookup
     //   either at the point of definition of the class template or member or
     //   during an instantiation of the class tem- plate or member.
-    if (!LookupInDependent &&
-        (BaseType->isDependentType() && !isCurrentInstantiation))
+    if (!LookupInDependent && BaseType->isDependentType())
       continue;
 
     // Determine whether we need to visit this base class at all,
@@ -251,8 +243,9 @@ bool CXXBasePaths::lookupInBases(ASTContext &Context,
         return FoundPath;
       }
     } else if (VisitBase) {
-      CXXRecordDecl *BaseRecord = nullptr;
+      CXXRecordDecl *BaseRecord;
       if (LookupInDependent) {
+        BaseRecord = nullptr;
         const TemplateSpecializationType *TST =
             BaseSpec.getType()->getAs<TemplateSpecializationType>();
         if (!TST) {
@@ -271,7 +264,8 @@ bool CXXBasePaths::lookupInBases(ASTContext &Context,
             BaseRecord = nullptr;
         }
       } else {
-        BaseRecord = cast<CXXRecordDecl>(BaseSpec.getType()->getAsRecordDecl());
+        BaseRecord = cast<CXXRecordDecl>(
+            BaseSpec.getType()->castAs<RecordType>()->getDecl());
       }
       if (BaseRecord &&
           lookupInBases(Context, BaseRecord, BaseMatches, LookupInDependent)) {
diff --git a/clang/test/CXX/drs/cwg5xx.cpp b/clang/test/CXX/drs/cwg5xx.cpp
index 0d53a9d07d76de..ed0c7159dfc889 100644
--- a/clang/test/CXX/drs/cwg5xx.cpp
+++ b/clang/test/CXX/drs/cwg5xx.cpp
@@ -1178,61 +1178,17 @@ namespace cwg590 { // cwg590: yes
   template<typename T> typename A<T>::B::C A<T>::B::C::f(A<T>::B::C) {}
 }
 
-namespace cwg591 { // cwg591: yes
+namespace cwg591 { // cwg591: no
   template<typename T> struct A {
     typedef int M;
     struct B {
       typedef void M;
       struct C;
-      struct D;
-    };
-  };
-
-  template<typename T> struct G {
-    struct B {
-      typedef int M;
-      struct C {
-        typedef void M;
-        struct D;
-      };
-    };
-  };
-
-  template<typename T> struct H {
-    template<typename U> struct B {
-      typedef int M;
-      template<typename F> struct C {
-        typedef void M;
-        struct D;
-        struct P;
-      };
     };
   };
 
   template<typename T> struct A<T>::B::C : A<T> {
-    M m;
-  };
-
-  template<typename T> struct G<T>::B::C::D : B {
-    M m;
-  };
-
-  template<typename T>
-  template<typename U>
-  template<typename F>
-  struct H<T>::B<U>::C<F>::D : B<U> {
-    M m;
-  };
-
-  template<typename T> struct A<T>::B::D : A<T*> {
-    M m;
-    // expected-error@-1 {{field has incomplete type 'M' (aka 'void'}}
-  };
-
-  template<typename T>
-  template<typename U>
-  template<typename F>
-  struct H<T>::B<U>::C<F>::P : B<F> {
+    // FIXME: Should find member of non-dependent base class A<T>.
     M m;
     // expected-error@-1 {{field has incomplete type 'M' (aka 'void'}}
   };
diff --git a/clang/www/cxx_dr_status.html b/clang/www/cxx_dr_status.html
index c773c58fac4d0f..186f7cc0ace546 100755
--- a/clang/www/cxx_dr_status.html
+++ b/clang/www/cxx_dr_status.html
@@ -3599,7 +3599,7 @@ <h2 id="cxxdr">C++ defect report implementation status</h2>
     <td><a href="https://cplusplus.github.io/CWG/issues/591.html">591</a></td>
     <td>CD4</td>
     <td>When a dependent base class is the current instantiation</td>
-    <td class="none" align="center">Yes</td>
+    <td class="none" align="center">No</td>
   </tr>
   <tr id="592">
     <td><a href="https://cplusplus.github.io/CWG/issues/592.html">592</a></td>

@asi-sc asi-sc merged commit f7dc1d0 into llvm:main Nov 26, 2024
7 of 10 checks passed
Copy link

⚠️ C/C++ code formatter, clang-format found issues in your code. ⚠️

You can test this locally with the following command:
git-clang-format --diff b214ca82daeece1568268ebc0fbcc2eaa649425b 7501e1cb92371d7082aa4e98ebe990d8f86bafe2 --extensions cpp -- clang/lib/AST/CXXInheritance.cpp clang/test/CXX/drs/cwg5xx.cpp
View the diff from clang-format here.
diff --git a/clang/lib/AST/CXXInheritance.cpp b/clang/lib/AST/CXXInheritance.cpp
index aefc06e919..b9e5071473 100644
--- a/clang/lib/AST/CXXInheritance.cpp
+++ b/clang/lib/AST/CXXInheritance.cpp
@@ -134,7 +134,7 @@ bool CXXRecordDecl::forallBases(ForallBasesCallback BaseMatches) const {
         return false;
 
       CXXRecordDecl *Base =
-            cast_or_null<CXXRecordDecl>(Ty->getDecl()->getDefinition());
+          cast_or_null<CXXRecordDecl>(Ty->getDecl()->getDefinition());
       if (!Base ||
           (Base->isDependentContext() &&
            !Base->isCurrentInstantiation(Record))) {

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
None yet
Development

Successfully merging this pull request may close these issues.

2 participants