[clang] [clang] fix infinite recursion (PR #143244)

[via cfe-commits]

llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-clang

Author: Zhikai Zeng (Backl1ght)

<details>
<summary>Changes</summary>

fix https://github.com/llvm/llvm-project/issues/141789

The direct cause of infinite recursion is  that `T` is changing from `struct X` and  `S<X>` infinitely,  this pr add a check that if  `T` visited before then return false directly.

```plaintext
/home/backlight/llvm-project/clang/lib/Sema/SemaDeclCXX.cpp:7196] FD->getType().getAsString()=struct X, T.getAsString()=S<X>, FD->getType().getCanonicalType().getUnqualifiedType().getAsString()=struct X, CanUnqualT.getAsString()=struct S<struct X>, 
/home/backlight/llvm-project/clang/lib/Sema/SemaDeclCXX.cpp:7196] FD->getType().getAsString()=S<X>, T.getAsString()=struct X, FD->getType().getCanonicalType().getUnqualifiedType().getAsString()=struct S<struct X>, CanUnqualT.getAsString()=struct X,
```

https://github.com/llvm/llvm-project/pull/104829 fix similar infinite recursion, but I think it is no longer needed so I kind of revert it.

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


2 Files Affected:

- (modified) clang/lib/Sema/SemaDeclCXX.cpp (+6-8) 
- (modified) clang/test/SemaCXX/gh102293.cpp (+17) 


``````````diff
diff --git a/clang/lib/Sema/SemaDeclCXX.cpp b/clang/lib/Sema/SemaDeclCXX.cpp
index 5ef2ae3ee857f..9f66c194a45dd 100644
--- a/clang/lib/Sema/SemaDeclCXX.cpp
+++ b/clang/lib/Sema/SemaDeclCXX.cpp
@@ -7172,7 +7172,10 @@ void Sema::CheckCompletedCXXClass(Scope *S, CXXRecordDecl *Record) {
     // "effectively constexpr" for better compatibility.
     // See https://github.com/llvm/llvm-project/issues/102293 for more info.
     if (isa<CXXDestructorDecl>(M)) {
-      auto Check = [](QualType T, auto &&Check) -> bool {
+      llvm::DenseSet<QualType> Visited;
+      auto Check = [&Visited](QualType T, auto &&Check) -> bool {
+        if (!Visited.insert(T).second)
+          return false;
         const CXXRecordDecl *RD =
             T->getBaseElementTypeUnsafe()->getAsCXXRecordDecl();
         if (!RD || !RD->isCompleteDefinition())
@@ -7181,16 +7184,11 @@ 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().getCanonicalType().getUnqualifiedType() !=
-                  CanUnqualT &&
-              !Check(B.getType(), Check))
+          if (!Check(B.getType(), Check))
             return false;
         for (const FieldDecl *FD : RD->fields())
-          if (FD->getType().getCanonicalType().getUnqualifiedType() !=
-                  CanUnqualT &&
-              !Check(FD->getType(), Check))
+          if (!Check(FD->getType(), Check))
             return false;
         return true;
       };
diff --git a/clang/test/SemaCXX/gh102293.cpp b/clang/test/SemaCXX/gh102293.cpp
index d4218cc13dcec..fe417e697841b 100644
--- a/clang/test/SemaCXX/gh102293.cpp
+++ b/clang/test/SemaCXX/gh102293.cpp
@@ -45,3 +45,20 @@ class quux : quux { // expected-error {{base class has incomplete type}} \
   virtual int c();
 };
 }
+
+// Ensure we don't get infinite recursion from the check, however. See GH141789
+namespace GH141789 {
+template <typename Ty>
+struct S {
+  Ty t; // expected-error {{field has incomplete type 'GH141789::X'}}
+};
+
+struct T {
+  ~T();
+};
+
+struct X { // expected-note {{definition of 'GH141789::X' is not complete until the closing '}'}}
+  S<X> next; // expected-note {{in instantiation of template class 'GH141789::S<GH141789::X>' requested here}}
+  T m;
+};
+}

``````````

</details>


https://github.com/llvm/llvm-project/pull/143244