[clang] Changed Checks from TriviallyCopyable to TriviallyCopyConstructible (PR #76680)

Bhuminjay Soni via cfe-commits cfe-commits at lists.llvm.org
Mon Jan 1 06:38:02 PST 2024


https://github.com/11happy created https://github.com/llvm/llvm-project/pull/76680

**Overview:**
This pull request fixes #47355 where in the Clang compiler's range-loop-analysis incorrectly checks for trivial copyability instead of trivial copy constructibility, leading to erroneous warnings.

**Changes Made:**
- The changes made in this commit address the issue by introducing a new member function `isTriviallyCopyConstructible` in the `CXXRecordDecl` class and implementing it in the associated files `(clang/include/clang/AST/DeclCXX.h, clang/lib/AST/DeclCXX.cpp)`. Additionally, modifications are made in `clang/include/clang/AST/Type.h` and `clang/lib/AST/Type.cpp` to support the new function. The implementation checks for trivial copy constructibility, distinguishing it from the existing `isTriviallyCopyable` check. The issue in `clang/lib/Sema/SemaStmt.cpp` is also addressed by updating the conditional check in the `DiagnoseForRangeConstVariableCopies` function to use the new `isTriviallyCopyConstructibleType` function. Overall, these changes aim to correct the range-loop-analysis by ensuring it checks for trivial copy constructibility rather than trivial copyability.

**Testing:**
- Tested the updated code.
- Verified that other functionalities remain unaffected.
![Screenshot from 2024-01-01 19-50-20](https://github.com/llvm/llvm-project/assets/76656712/c664f01a-522e-45e4-8363-39f13d8cc241)


**Dependencies:**
- No dependencies on other pull requests.

**References:**
- https://cplusplus.com/reference/type_traits/is_trivially_copy_constructible/
- https://en.cppreference.com/w/cpp/named_req/CopyConstructible
- https://cplusplus.com/reference/type_traits/is_trivially_copyable/

**CC:**
- @Endilll , @r4nt , @AaronBallman 


>From 3c8dfcf96e732f4546f8019c0111fd873b233162 Mon Sep 17 00:00:00 2001
From: 11happy <soni5happy at gmail.com>
Date: Mon, 1 Jan 2024 19:53:45 +0530
Subject: [PATCH] Changed Checks from TriviallyCopyable to
 TriviallyCopyConstructible

Signed-off-by: 11happy <soni5happy at gmail.com>
---
 clang/include/clang/AST/DeclCXX.h |  3 +++
 clang/include/clang/AST/Type.h    |  3 +++
 clang/lib/AST/DeclCXX.cpp         | 13 ++++++++++
 clang/lib/AST/Type.cpp            | 43 +++++++++++++++++++++++++++++++
 clang/lib/Sema/SemaStmt.cpp       |  2 +-
 5 files changed, 63 insertions(+), 1 deletion(-)

diff --git a/clang/include/clang/AST/DeclCXX.h b/clang/include/clang/AST/DeclCXX.h
index 432293583576b5..3ac0d6a9e083cd 100644
--- a/clang/include/clang/AST/DeclCXX.h
+++ b/clang/include/clang/AST/DeclCXX.h
@@ -1425,6 +1425,9 @@ class CXXRecordDecl : public RecordDecl {
   /// (C++11 [class]p6).
   bool isTriviallyCopyable() const;
 
+  /// Determine whether this class is considered trivially copyable per
+  bool isTriviallyCopyConstructible() const;
+
   /// Determine whether this class is considered trivial.
   ///
   /// C++11 [class]p6:
diff --git a/clang/include/clang/AST/Type.h b/clang/include/clang/AST/Type.h
index 1afa693672860f..bce2256f96a828 100644
--- a/clang/include/clang/AST/Type.h
+++ b/clang/include/clang/AST/Type.h
@@ -917,6 +917,9 @@ class QualType {
   /// Return true if this is a trivially copyable type (C++0x [basic.types]p9)
   bool isTriviallyCopyableType(const ASTContext &Context) const;
 
+  /// Return true if this is a trivially copyable type
+  bool isTriviallyCopyConstructibleType(const ASTContext &Context) const;
+
   /// Return true if this is a trivially relocatable type.
   bool isTriviallyRelocatableType(const ASTContext &Context) const;
 
diff --git a/clang/lib/AST/DeclCXX.cpp b/clang/lib/AST/DeclCXX.cpp
index c944862fcefeee..98b0a6dc28ea2f 100644
--- a/clang/lib/AST/DeclCXX.cpp
+++ b/clang/lib/AST/DeclCXX.cpp
@@ -587,6 +587,19 @@ bool CXXRecordDecl::isTriviallyCopyable() const {
   return true;
 }
 
+bool CXXRecordDecl::isTriviallyCopyConstructible() const {
+
+  //   A trivially copy constructible class is a class that:
+  //   -- has no non-trivial copy constructors,
+  if (hasNonTrivialCopyConstructor())
+    return false;
+  //   -- has a trivial destructor.
+  if (!hasTrivialDestructor())
+    return false;
+
+  return true;
+}
+
 void CXXRecordDecl::markedVirtualFunctionPure() {
   // C++ [class.abstract]p2:
   //   A class is abstract if it has at least one pure virtual function.
diff --git a/clang/lib/AST/Type.cpp b/clang/lib/AST/Type.cpp
index 160a725939ccd4..9c8b25798a0a95 100644
--- a/clang/lib/AST/Type.cpp
+++ b/clang/lib/AST/Type.cpp
@@ -2644,6 +2644,49 @@ bool QualType::isTriviallyCopyableType(const ASTContext &Context) const {
   return false;
 }
 
+bool QualType::isTriviallyCopyConstructibleType(
+    const ASTContext &Context) const {
+  if ((*this)->isArrayType())
+    return Context.getBaseElementType(*this).isTriviallyCopyConstructibleType(
+        Context);
+
+  if (hasNonTrivialObjCLifetime())
+    return false;
+
+  // C++11 [basic.types]p9 - See Core 2094
+  //   Scalar types, trivially copyable class types, arrays of such types, and
+  //   cv-qualified versions of these types are collectively
+  //   called trivially copy constructible types.
+
+  QualType CanonicalType = getCanonicalType();
+  if (CanonicalType->isDependentType())
+    return false;
+
+  if (CanonicalType->isSizelessBuiltinType())
+    return true;
+
+  // Return false for incomplete types after skipping any incomplete array types
+  // which are expressly allowed by the standard and thus our API.
+  if (CanonicalType->isIncompleteType())
+    return false;
+
+  // As an extension, Clang treats vector types as Scalar types.
+  if (CanonicalType->isScalarType() || CanonicalType->isVectorType())
+    return true;
+
+  if (const auto *RT = CanonicalType->getAs<RecordType>()) {
+    if (const auto *ClassDecl = dyn_cast<CXXRecordDecl>(RT->getDecl())) {
+      if (!ClassDecl->isTriviallyCopyConstructible())
+        return false;
+    }
+
+    return true;
+  }
+
+  // No other types can match.
+  return false;
+}
+
 bool QualType::isTriviallyRelocatableType(const ASTContext &Context) const {
   QualType BaseElementType = Context.getBaseElementType(*this);
 
diff --git a/clang/lib/Sema/SemaStmt.cpp b/clang/lib/Sema/SemaStmt.cpp
index f0b03db690843a..21efe25ed84a3d 100644
--- a/clang/lib/Sema/SemaStmt.cpp
+++ b/clang/lib/Sema/SemaStmt.cpp
@@ -3200,7 +3200,7 @@ static void DiagnoseForRangeConstVariableCopies(Sema &SemaRef,
   // (The function `getTypeSize` returns the size in bits.)
   ASTContext &Ctx = SemaRef.Context;
   if (Ctx.getTypeSize(VariableType) <= 64 * 8 &&
-      (VariableType.isTriviallyCopyableType(Ctx) ||
+      (VariableType.isTriviallyCopyConstructibleType(Ctx) ||
        hasTrivialABIAttr(VariableType)))
     return;
 



More information about the cfe-commits mailing list