[Clang] Fix a regression introduced by #161163.

[cor3ntin]

Classes with a user provided constructor are still implicit lifetime if they have an implicit, trivial copy ctr.

[llvmbot]

@llvm/pr-subscribers-clang

Author: Corentin Jabot (cor3ntin)

Changes

Classes with a user provided constructor are still implicit lifetime if they have an implicit, trivial copy ctr.

---

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

2 Files Affected:

• (modified) clang/lib/Sema/SemaTypeTraits.cpp (+17-7)
• (modified) clang/test/SemaCXX/type-traits.cpp (+11-1)

diff --git a/clang/lib/Sema/SemaTypeTraits.cpp b/clang/lib/Sema/SemaTypeTraits.cpp
index 3e34675cbf064..13f25c453d8ba 100644
--- a/clang/lib/Sema/SemaTypeTraits.cpp
+++ b/clang/lib/Sema/SemaTypeTraits.cpp
@@ -1165,14 +1165,24 @@ static bool EvaluateUnaryTypeTrait(Sema &Self, TypeTrait UTT,
     const CXXDestructorDecl *Dtor = RD->getDestructor();
     if (UnqualT->isAggregateType() && (!Dtor || !Dtor->isUserProvided()))
       return true;
-    if (RD->hasTrivialDestructor() && (!Dtor || !Dtor->isDeleted())) {
-      for (CXXConstructorDecl *Ctr : RD->ctors()) {
-        if (Ctr->isIneligibleOrNotSelected() || Ctr->isDeleted())
-          continue;
-        if (Ctr->isTrivial())
-          return true;
-      }
+    if (!(RD->hasTrivialDestructor() && (!Dtor || !Dtor->isDeleted())))
+      return false;
+    bool FoundCopyCtr = false;
+    bool FoundMoveCtr = false;
+    for (CXXConstructorDecl *Ctr : RD->ctors()) {
+      FoundCopyCtr = Ctr->isCopyConstructor();
+      FoundMoveCtr = Ctr->isMoveConstructor();
+      if (Ctr->isIneligibleOrNotSelected() || Ctr->isDeleted())
+        continue;
+      if (Ctr->isTrivial())
+        return true;
     }
+    if (!FoundCopyCtr && RD->hasTrivialCopyConstructor() &&
+        !RD->defaultedCopyConstructorIsDeleted())
+      return true;
+    if (!FoundMoveCtr && RD->hasTrivialMoveConstructor() &&
+        !RD->defaultedMoveConstructorIsDeleted())
+      return true;
     return false;
   }
   case UTT_IsIntangibleType:
diff --git a/clang/test/SemaCXX/type-traits.cpp b/clang/test/SemaCXX/type-traits.cpp
index 901d510bba847..343529fe81b57 100644
--- a/clang/test/SemaCXX/type-traits.cpp
+++ b/clang/test/SemaCXX/type-traits.cpp
@@ -2066,7 +2066,17 @@ class UserProvidedConstructor {
     UserProvidedConstructor(const UserProvidedConstructor&)            = delete;
     UserProvidedConstructor& operator=(const UserProvidedConstructor&) = delete;
 };
+struct Ctr {
+    Ctr();
+};
+struct Ctr2 {
+    Ctr2();
+private:
+  NoEligibleTrivialContructor inner;
+};
 
+static_assert(__builtin_is_implicit_lifetime(Ctr));
+static_assert(!__builtin_is_implicit_lifetime(NoEligibleTrivialContructor));
 static_assert(__builtin_is_implicit_lifetime(NonAggregate));
 static_assert(!__builtin_is_implicit_lifetime(DataMemberInitializer));
 static_assert(!__builtin_is_implicit_lifetime(UserProvidedConstructor));
@@ -2076,7 +2086,7 @@ template <typename T>
 class Tpl {
     Tpl() requires false = default ;
 };
-static_assert(!__builtin_is_implicit_lifetime(Tpl<int>));
+static_assert(__builtin_is_implicit_lifetime(Tpl<int>));
 
 #endif
 }

[bolshakov-a]

What about implicit default constructors, like in this case?

struct NonCopyable{
    NonCopyable() = default;
    NonCopyable(const NonCopyable&) = delete;
};

class C {
    NonCopyable nc;
};

//C c;

static_assert(__builtin_is_implicit_lifetime(C));

[bolshakov-a]

The test fails now because short-circuiting here occurs before FoundCopyCtr can be set.

[bolshakov-a]

This is never reached because RD->hasTrivialDefaultConstructor() == true case has been short-circuited above.

[bolshakov-a]

It would be good to add a note that the classes should not be used here in any way causing declarations of the implicit methods to be instantiated (or whatever the correct word is).

[bolshakov-a]

Ctr2 is not used in this test. Maybe, you wanted to mention it here instead of NoEligibleTrivialContructor?

[bolshakov-a]

UTT_HasTrivialDefaultConstructor handler checks RD->hasTrivialDefaultConstructor() && !RD->hasNonTrivialDefaultConstructor() for some reason. Not sure if this is really needed.

[bolshakov-a]

Now LGTM, thanks! Btw, do you know what !RD->hasNonTrivialDefaultConstructor() check is needed for? If you do, it would be good to add a corresponding test case. If not, a TODO to investigate it is worth placing in the code.

[cor3ntin]

hasNonTrivialDefaultConstructor

you can have

template <typename>
class C {
  C();
  C() requires true;
};

But in the case of __builtin_is_implicit_lifetime we probably want to allow that, actually...

[bolshakov-a]

Looks like your new test cases with MultipleDefaults pass even on the previous code revision (with !RD->hasNonTrivialDefaultConstructor() call).