[clang] [C++23] Fix infinite recursion (Clang 19.x regression) (PR #104829)

[Aaron Ballman via cfe-commits]

https://github.com/AaronBallman updated https://github.com/llvm/llvm-project/pull/104829

>From 4d40ac342c3dfa48901a92c4ae5f70ba356d1768 Mon Sep 17 00:00:00 2001
From: Aaron Ballman <aaron at aaronballman.com>
Date: Mon, 19 Aug 2024 14:28:48 -0400
Subject: [PATCH 1/2] [C++23] Fix infinite recursion (Clang 19.x regression)

d469794d0cdfd2fea50a6ce0c0e33abb242d744c 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 #104802
---
 clang/lib/Sema/SemaDeclCXX.cpp  |  4 ++--
 clang/test/SemaCXX/gh102293.cpp | 13 ++++++++++++-
 2 files changed, 14 insertions(+), 3 deletions(-)

diff --git a/clang/lib/Sema/SemaDeclCXX.cpp b/clang/lib/Sema/SemaDeclCXX.cpp
index e05595e565d54a..c1fc61c263791a 100644
--- a/clang/lib/Sema/SemaDeclCXX.cpp
+++ b/clang/lib/Sema/SemaDeclCXX.cpp
@@ -7057,10 +7057,10 @@ void Sema::CheckCompletedCXXClass(Scope *S, CXXRecordDecl *Record) {
           return false;
 
         for (const CXXBaseSpecifier &B : RD->bases())
-          if (!Check(B.getType(), Check))
+          if (B.getType() != T && !Check(B.getType(), Check))
             return false;
         for (const FieldDecl *FD : RD->fields())
-          if (!Check(FD->getType(), Check))
+          if (FD->getType() != T && !Check(FD->getType(), Check))
             return false;
         return true;
       };
diff --git a/clang/test/SemaCXX/gh102293.cpp b/clang/test/SemaCXX/gh102293.cpp
index 30629fc03bf6a9..ece9dfd21da11c 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,15 @@ 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 : bar { // expected-error {{base class has incomplete type}} \
+                     expected-note {{definition of 'GH104802::bar' is not complete until the closing '}'}}
+  virtual int c();
+};
+}

>From 9e4f91ab6581935c2f4f25c90f11cf9d4584fa77 Mon Sep 17 00:00:00 2001
From: Aaron Ballman <aaron at aaronballman.com>
Date: Mon, 19 Aug 2024 14:57:11 -0400
Subject: [PATCH 2/2] Add test coverage for qualified types and non-canonical
 types

The original fix did not go far enough, as testing found out.
---
 clang/lib/Sema/SemaDeclCXX.cpp  |  9 +++++++--
 clang/test/SemaCXX/gh102293.cpp | 18 ++++++++++++++++--
 2 files changed, 23 insertions(+), 4 deletions(-)

diff --git a/clang/lib/Sema/SemaDeclCXX.cpp b/clang/lib/Sema/SemaDeclCXX.cpp
index c1fc61c263791a..d89a47f3e6226a 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 (B.getType() != T && !Check(B.getType(), Check))
+          if (B.getType().getCanonicalType().getUnqualifiedType() !=
+                  CanUnqualT &&
+              !Check(B.getType(), Check))
             return false;
         for (const FieldDecl *FD : RD->fields())
-          if (FD->getType() != T && !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 ece9dfd21da11c..d4218cc13dcecd 100644
--- a/clang/test/SemaCXX/gh102293.cpp
+++ b/clang/test/SemaCXX/gh102293.cpp
@@ -23,11 +23,25 @@ struct S : HasVT {
 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 bar : bar { // expected-error {{base class has incomplete type}} \
-                     expected-note {{definition of 'GH104802::bar' is not complete until the closing '}'}}
+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();
 };
 }