[clang] efcf192 - Changed Checks from TriviallyCopyable to TriviallyCopyConstructible (#77194)
via cfe-commits
cfe-commits at lists.llvm.org
Tue Jan 9 23:28:03 PST 2024
Author: Bhuminjay Soni
Date: 2024-01-10T08:27:58+01:00
New Revision: efcf192a0a5993165f837ce71250fb6df689634b
URL: https://github.com/llvm/llvm-project/commit/efcf192a0a5993165f837ce71250fb6df689634b
DIFF: https://github.com/llvm/llvm-project/commit/efcf192a0a5993165f837ce71250fb6df689634b.diff
LOG: 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
Added:
Modified:
clang/docs/ReleaseNotes.rst
clang/include/clang/AST/DeclCXX.h
clang/include/clang/AST/Type.h
clang/lib/AST/DeclCXX.cpp
clang/lib/AST/Type.cpp
clang/lib/Sema/SemaStmt.cpp
clang/test/SemaCXX/warn-range-loop-analysis-trivially-copyable.cpp
Removed:
################################################################################
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 46f4b82b89e488..15479906d22bdd 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 984a4d8bab5e77..648f5f94640870 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 9e9f896ebef7a6..d4e5310fb3abc6 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 a894d3289eb185..b419fc8836b032 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 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;
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 e345ef40aed9ca..8fa387908c3658 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 at +3 {{loop variable 'r' creates a copy from type 'const Record'}}
+ // expected-note at +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;
}
+
More information about the cfe-commits
mailing list