diff options
author | Bhuminjay Soni <76656712+11happy@users.noreply.github.com> | 2024-01-10 12:57:58 +0530 |
---|---|---|
committer | GitHub <noreply@github.com> | 2024-01-10 08:27:58 +0100 |
commit | efcf192a0a5993165f837ce71250fb6df689634b (patch) | |
tree | acea2f3bb65ac2beb97ce9d46d3f5a4ba862b682 | |
parent | 7fc7ef14340a3a58cebd0801497b68eb698c2784 (diff) | |
download | llvm-efcf192a0a5993165f837ce71250fb6df689634b.zip llvm-efcf192a0a5993165f837ce71250fb6df689634b.tar.gz llvm-efcf192a0a5993165f837ce71250fb6df689634b.tar.bz2 |
Changed Checks from TriviallyCopyable to TriviallyCopyConstructible (#77194)
**Overview:**
Fix a bug where Clang's range-loop-analysis incorrectly checks for trivial copyability instead
of trivial copy constructibility, leading to erroneous warnings.
Fixes #47355
-rw-r--r-- | clang/docs/ReleaseNotes.rst | 3 | ||||
-rw-r--r-- | clang/include/clang/AST/DeclCXX.h | 3 | ||||
-rw-r--r-- | clang/include/clang/AST/Type.h | 3 | ||||
-rw-r--r-- | clang/lib/AST/DeclCXX.cpp | 13 | ||||
-rw-r--r-- | clang/lib/AST/Type.cpp | 34 | ||||
-rw-r--r-- | clang/lib/Sema/SemaStmt.cpp | 2 | ||||
-rw-r--r-- | clang/test/SemaCXX/warn-range-loop-analysis-trivially-copyable.cpp | 25 |
7 files changed, 73 insertions, 10 deletions
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst index 46f4b82..1547990 100644 --- a/clang/docs/ReleaseNotes.rst +++ b/clang/docs/ReleaseNotes.rst @@ -884,6 +884,9 @@ Bug Fixes to AST Handling - Fixed a bug where RecursiveASTVisitor fails to visit the initializer of a bitfield. `Issue 64916 <https://github.com/llvm/llvm-project/issues/64916>`_ +- Fixed a bug where range-loop-analysis checks for trivial copyability, + rather than trivial copy-constructibility + `Issue 47355 <https://github.com/llvm/llvm-project/issues/47355>`_ - Fixed a bug where Template Instantiation failed to handle Lambda Expressions with certain types of Attributes. (`#76521 <https://github.com/llvm/llvm-project/issues/76521>`_) diff --git a/clang/include/clang/AST/DeclCXX.h b/clang/include/clang/AST/DeclCXX.h index 984a4d8..648f5f9 100644 --- a/clang/include/clang/AST/DeclCXX.h +++ b/clang/include/clang/AST/DeclCXX.h @@ -1425,6 +1425,9 @@ public: /// (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 9e9f896..d4e5310 100644 --- a/clang/include/clang/AST/Type.h +++ b/clang/include/clang/AST/Type.h @@ -917,6 +917,9 @@ public: /// 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 c944862..98b0a6d 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 a894d32..b419fc8 100644 --- a/clang/lib/AST/Type.cpp +++ b/clang/lib/AST/Type.cpp @@ -2604,19 +2604,22 @@ bool QualType::isTrivialType(const ASTContext &Context) const { return false; } -bool QualType::isTriviallyCopyableType(const ASTContext &Context) const { - if ((*this)->isArrayType()) - return Context.getBaseElementType(*this).isTriviallyCopyableType(Context); +static bool isTriviallyCopyableTypeImpl(const QualType &type, + const ASTContext &Context, + bool IsCopyConstructible) { + if (type->isArrayType()) + return isTriviallyCopyableTypeImpl(Context.getBaseElementType(type), + Context, IsCopyConstructible); - if (hasNonTrivialObjCLifetime()) + if (type.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 copyable types. + // called trivially copy constructible types. - QualType CanonicalType = getCanonicalType(); + QualType CanonicalType = type.getCanonicalType(); if (CanonicalType->isDependentType()) return false; @@ -2634,16 +2637,29 @@ bool QualType::isTriviallyCopyableType(const ASTContext &Context) const { if (const auto *RT = CanonicalType->getAs<RecordType>()) { if (const auto *ClassDecl = dyn_cast<CXXRecordDecl>(RT->getDecl())) { - if (!ClassDecl->isTriviallyCopyable()) return false; + if (IsCopyConstructible) { + return ClassDecl->isTriviallyCopyConstructible(); + } else { + return ClassDecl->isTriviallyCopyable(); + } } - return true; } - // No other types can match. return false; } +bool QualType::isTriviallyCopyableType(const ASTContext &Context) const { + return isTriviallyCopyableTypeImpl(*this, Context, + /*IsCopyConstructible=*/false); +} + +bool QualType::isTriviallyCopyConstructibleType( + const ASTContext &Context) const { + return isTriviallyCopyableTypeImpl(*this, Context, + /*IsCopyConstructible=*/true); +} + 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 f0b03db..21efe25 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; diff --git a/clang/test/SemaCXX/warn-range-loop-analysis-trivially-copyable.cpp b/clang/test/SemaCXX/warn-range-loop-analysis-trivially-copyable.cpp index e345ef4..8fa3879 100644 --- a/clang/test/SemaCXX/warn-range-loop-analysis-trivially-copyable.cpp +++ b/clang/test/SemaCXX/warn-range-loop-analysis-trivially-copyable.cpp @@ -33,6 +33,17 @@ void test_TriviallyCopyable_64_bytes() { for (const auto r : records) (void)r; } +void test_TriviallyCopyConstructible_64_bytes() { + struct Record { + char a[64]; + Record& operator=(Record const& other){return *this;}; + + }; + + Record records[8]; + for (const auto r : records) + (void)r; +} void test_TriviallyCopyable_65_bytes() { struct Record { @@ -47,6 +58,19 @@ void test_TriviallyCopyable_65_bytes() { (void)r; } +void test_TriviallyCopyConstructible_65_bytes() { + struct Record { + char a[65]; + Record& operator=(Record const& other){return *this;}; + + }; + // expected-warning@+3 {{loop variable 'r' creates a copy from type 'const Record'}} + // expected-note@+2 {{use reference type 'const Record &' to prevent copying}} + Record records[8]; + for (const auto r : records) + (void)r; +} + void test_NonTriviallyCopyable() { struct Record { Record() {} @@ -87,3 +111,4 @@ void test_TrivialABI_65_bytes() { for (const auto r : records) (void)r; } + |