Skip to content

Conversation

llvmbot
Copy link
Member

@llvmbot llvmbot commented Aug 19, 2024

Backport 435cb0d

Requested by: @AaronBallman

@llvmbot llvmbot added this to the LLVM 19.X Release milestone Aug 19, 2024
@llvmbot
Copy link
Member Author

llvmbot commented Aug 19, 2024

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

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

llvmbot commented Aug 19, 2024

@llvm/pr-subscribers-clang

Author: None (llvmbot)

Changes

Backport 435cb0d

Requested by: @AaronBallman


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

2 Files Affected:

  • (modified) clang/lib/Sema/SemaDeclCXX.cpp (+7-2)
  • (modified) clang/test/SemaCXX/gh102293.cpp (+26-1)
diff --git a/clang/lib/Sema/SemaDeclCXX.cpp b/clang/lib/Sema/SemaDeclCXX.cpp
index ecf8754143a49e..92c47be67339e9 100644
--- a/clang/lib/Sema/SemaDeclCXX.cpp
+++ b/clang/lib/Sema/SemaDeclCXX.cpp
@@ -7056,11 +7056,16 @@ void Sema::CheckCompletedCXXClass(Scope *S, CXXRecordDecl *Record) {
         if (!RD->hasConstexprDestructor())
           return false;
 
+        QualType CanUnqualT = T.getCanonicalType().getUnqualifiedType();
         for (const CXXBaseSpecifier &B : RD->bases())
-          if (!Check(B.getType(), Check))
+          if (B.getType().getCanonicalType().getUnqualifiedType() !=
+                  CanUnqualT &&
+              !Check(B.getType(), Check))
             return false;
         for (const FieldDecl *FD : RD->fields())
-          if (!Check(FD->getType(), Check))
+          if (FD->getType().getCanonicalType().getUnqualifiedType() !=
+                  CanUnqualT &&
+              !Check(FD->getType(), Check))
             return false;
         return true;
       };
diff --git a/clang/test/SemaCXX/gh102293.cpp b/clang/test/SemaCXX/gh102293.cpp
index 30629fc03bf6a9..d4218cc13dcecd 100644
--- a/clang/test/SemaCXX/gh102293.cpp
+++ b/clang/test/SemaCXX/gh102293.cpp
@@ -1,5 +1,4 @@
 // RUN: %clang_cc1 -std=c++23 -fsyntax-only -verify %s
-// expected-no-diagnostics
 
 template <typename T> static void destroy() {
     T t;
@@ -20,3 +19,29 @@ struct S : HasVT {
   HasD<> v;
 };
 
+// Ensure we don't get infinite recursion from the check, however. See GH104802
+namespace GH104802 {
+class foo {       // expected-note {{definition of 'GH104802::foo' is not complete until the closing '}'}}
+  foo a;          // expected-error {{field has incomplete type 'foo'}}
+
+  virtual int c();
+};
+
+class bar {       // expected-note {{definition of 'GH104802::bar' is not complete until the closing '}'}}
+  const bar a;    // expected-error {{field has incomplete type 'const bar'}}
+
+  virtual int c();
+};
+
+class baz {       // expected-note {{definition of 'GH104802::baz' is not complete until the closing '}'}}
+  typedef class baz blech;
+  blech a;        // expected-error {{field has incomplete type 'blech' (aka 'GH104802::baz')}}
+
+  virtual int c();
+};
+
+class quux : quux { // expected-error {{base class has incomplete type}} \
+                     expected-note {{definition of 'GH104802::quux' is not complete until the closing '}'}}
+  virtual int c();
+};
+}

d469794 was fixing an issue with
triggering vtable instantiations, but it accidentally introduced
infinite recursion when the type to be checked is the same as the type
used in a base specifier or field declaration.

Fixes llvm#104802

(cherry picked from commit 435cb0d)
@tru tru merged commit 3ffa542 into llvm:release/19.x Aug 20, 2024
8 of 9 checks passed
Copy link

@AaronBallman (or anyone else). If you would like to add a note about this fix in the release notes (completely optional). Please reply to this comment with a one or two sentence description of the fix. When you are done, please add the release:note label to this PR.

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