Skip to content

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
@github-actions
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