[clang-tools-extra] 311091e - [clang-tidy] Fix crash in `modernize-use-default-member-init`
Piotr Zegar via cfe-commits
cfe-commits at lists.llvm.org
Tue Jun 13 11:14:13 PDT 2023
Author: AMS21
Date: 2023-06-13T18:13:52Z
New Revision: 311091e2b007ebe0da9877953a9a56a51102e60d
URL: https://github.com/llvm/llvm-project/commit/311091e2b007ebe0da9877953a9a56a51102e60d
DIFF: https://github.com/llvm/llvm-project/commit/311091e2b007ebe0da9877953a9a56a51102e60d.diff
LOG: [clang-tidy] Fix crash in `modernize-use-default-member-init`
This was causes by `getValueOfValueInit` unconditionally calling
`getScalarTypeKind` on the member type, which would then trigger an
assertions since arrays are not scalar type.
This fixes llvm#63285
Reviewed By: PiotrZSL
Differential Revision: https://reviews.llvm.org/D152802
Added:
Modified:
clang-tools-extra/clang-tidy/modernize/UseDefaultMemberInitCheck.cpp
clang-tools-extra/docs/ReleaseNotes.rst
clang-tools-extra/test/clang-tidy/checkers/modernize/use-default-member-init-assignment.cpp
clang-tools-extra/test/clang-tidy/checkers/modernize/use-default-member-init.cpp
Removed:
################################################################################
diff --git a/clang-tools-extra/clang-tidy/modernize/UseDefaultMemberInitCheck.cpp b/clang-tools-extra/clang-tidy/modernize/UseDefaultMemberInitCheck.cpp
index 673c475db630f..6c06b0af342f6 100644
--- a/clang-tools-extra/clang-tidy/modernize/UseDefaultMemberInitCheck.cpp
+++ b/clang-tools-extra/clang-tidy/modernize/UseDefaultMemberInitCheck.cpp
@@ -206,7 +206,7 @@ void UseDefaultMemberInitCheck::registerMatchers(MatchFinder *Finder) {
auto Init =
anyOf(initListExpr(anyOf(allOf(initCountIs(1), hasInit(0, InitBase)),
- initCountIs(0))),
+ initCountIs(0), hasType(arrayType()))),
InitBase);
Finder->addMatcher(
@@ -267,20 +267,30 @@ void UseDefaultMemberInitCheck::checkDefaultInit(
CharSourceRange InitRange =
CharSourceRange::getCharRange(LParenEnd, Init->getRParenLoc());
- bool ValueInit = isa<ImplicitValueInitExpr>(Init->getInit());
- bool CanAssign = UseAssignment && (!ValueInit || !Init->getInit()->getType()->isEnumeralType());
+ const Expr *InitExpression = Init->getInit();
+ const QualType InitType = InitExpression->getType();
+
+ const bool ValueInit =
+ isa<ImplicitValueInitExpr>(InitExpression) && !isa<ArrayType>(InitType);
+ const bool CanAssign =
+ UseAssignment && (!ValueInit || !InitType->isEnumeralType());
+ const bool NeedsBraces = !CanAssign || isa<ArrayType>(InitType);
auto Diag =
diag(Field->getLocation(), "use default member initializer for %0")
- << Field
- << FixItHint::CreateInsertion(FieldEnd, CanAssign ? " = " : "{")
- << FixItHint::CreateInsertionFromRange(FieldEnd, InitRange);
+ << Field;
+
+ if (CanAssign)
+ Diag << FixItHint::CreateInsertion(FieldEnd, " = ");
+ if (NeedsBraces)
+ Diag << FixItHint::CreateInsertion(FieldEnd, "{");
if (CanAssign && ValueInit)
- Diag << FixItHint::CreateInsertion(
- FieldEnd, getValueOfValueInit(Init->getInit()->getType()));
+ Diag << FixItHint::CreateInsertion(FieldEnd, getValueOfValueInit(InitType));
+ else
+ Diag << FixItHint::CreateInsertionFromRange(FieldEnd, InitRange);
- if (!CanAssign)
+ if (NeedsBraces)
Diag << FixItHint::CreateInsertion(FieldEnd, "}");
Diag << FixItHint::CreateRemoval(Init->getSourceRange());
@@ -294,8 +304,7 @@ void UseDefaultMemberInitCheck::checkExistingInit(
return;
diag(Init->getSourceLocation(), "member initializer for %0 is redundant")
- << Field
- << FixItHint::CreateRemoval(Init->getSourceRange());
+ << Field << FixItHint::CreateRemoval(Init->getSourceRange());
}
} // namespace clang::tidy::modernize
diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst
index cb9b961e92448..a0b996f51109d 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -325,6 +325,10 @@ Changes in existing checks
constructors toward hand written constructors so that they are skipped if more
than one exists.
+- Fixed crash in :doc:`modernize-use-default-member-init
+ <clang-tidy/checks/modernize/use-default-member-init>` with array members which
+ are value initialized.
+
- Fixed false positive in :doc:`modernize-use-equals-default
<clang-tidy/checks/modernize/use-equals-default>` check for special member
functions containing macros or preprocessor directives, and out-of-line special
diff --git a/clang-tools-extra/test/clang-tidy/checkers/modernize/use-default-member-init-assignment.cpp b/clang-tools-extra/test/clang-tidy/checkers/modernize/use-default-member-init-assignment.cpp
index a49d7a79522bd..4fdd21294d749 100644
--- a/clang-tools-extra/test/clang-tidy/checkers/modernize/use-default-member-init-assignment.cpp
+++ b/clang-tools-extra/test/clang-tidy/checkers/modernize/use-default-member-init-assignment.cpp
@@ -190,3 +190,39 @@ struct NegativeTemplate {
NegativeTemplate<int> nti;
NegativeTemplate<double> ntd;
+
+namespace PR63285 {
+
+class ArrayValueInit {
+ ArrayValueInit() : m_array() {}
+ double m_array[1];
+ // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: use default member initializer for 'm_array' [modernize-use-default-member-init]
+ // CHECK-FIXES: {{^ }}ArrayValueInit() {}
+ // CHECK-FIXES-NEXT: {{^ }}double m_array[1] = {};
+};
+
+class ArrayBraceInit {
+ ArrayBraceInit() : m_array{} {}
+ double m_array[1];
+ // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: use default member initializer for 'm_array' [modernize-use-default-member-init]
+ // CHECK-FIXES: {{^ }}ArrayBraceInit() {}
+ // CHECK-FIXES-NEXT: {{^ }}double m_array[1] = {};
+};
+
+class ArrayBraceInitWithValue {
+ ArrayBraceInitWithValue() : m_array{3.14} {}
+ double m_array[1];
+ // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: use default member initializer for 'm_array' [modernize-use-default-member-init]
+ // CHECK-FIXES: {{^ }}ArrayBraceInitWithValue() {}
+ // CHECK-FIXES-NEXT: {{^ }}double m_array[1] = {3.14};
+};
+
+class ArrayBraceInitMultipleValues {
+ ArrayBraceInitMultipleValues() : m_array{1.0, 2.0, 3.0} {}
+ double m_array[3];
+ // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: use default member initializer for 'm_array' [modernize-use-default-member-init]
+ // CHECK-FIXES: {{^ }}ArrayBraceInitMultipleValues() {}
+ // CHECK-FIXES-NEXT: {{^ }}double m_array[3] = {1.0, 2.0, 3.0};
+};
+
+} // namespace PR63285
diff --git a/clang-tools-extra/test/clang-tidy/checkers/modernize/use-default-member-init.cpp b/clang-tools-extra/test/clang-tidy/checkers/modernize/use-default-member-init.cpp
index 3eb6331d0a9f2..81c980e0217e6 100644
--- a/clang-tools-extra/test/clang-tidy/checkers/modernize/use-default-member-init.cpp
+++ b/clang-tools-extra/test/clang-tidy/checkers/modernize/use-default-member-init.cpp
@@ -482,3 +482,39 @@ struct EmptyBracedIntDefault {
// CHECK-FIXES: {{^ }}EmptyBracedIntDefault() {}
// CHECK-FIXES-NEXT: {{^ }}int m_i{};
};
+
+namespace PR63285 {
+
+class ArrayValueInit {
+ ArrayValueInit() : m_array() {}
+ double m_array[1];
+ // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: use default member initializer for 'm_array' [modernize-use-default-member-init]
+ // CHECK-FIXES: {{^ }}ArrayValueInit() {}
+ // CHECK-FIXES-NEXT: {{^ }}double m_array[1]{};
+};
+
+class ArrayBraceInit {
+ ArrayBraceInit() : m_array{} {}
+ double m_array[1];
+ // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: use default member initializer for 'm_array' [modernize-use-default-member-init]
+ // CHECK-FIXES: {{^ }}ArrayBraceInit() {}
+ // CHECK-FIXES-NEXT: {{^ }}double m_array[1]{};
+};
+
+class ArrayBraceInitWithValue {
+ ArrayBraceInitWithValue() : m_array{3.14} {}
+ double m_array[1];
+ // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: use default member initializer for 'm_array' [modernize-use-default-member-init]
+ // CHECK-FIXES: {{^ }}ArrayBraceInitWithValue() {}
+ // CHECK-FIXES-NEXT: {{^ }}double m_array[1]{3.14};
+};
+
+class ArrayBraceInitMultipleValues {
+ ArrayBraceInitMultipleValues() : m_array{1.0, 2.0, 3.0} {}
+ double m_array[3];
+ // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: use default member initializer for 'm_array' [modernize-use-default-member-init]
+ // CHECK-FIXES: {{^ }}ArrayBraceInitMultipleValues() {}
+ // CHECK-FIXES-NEXT: {{^ }}double m_array[3]{1.0, 2.0, 3.0};
+};
+
+} // namespace PR63285
More information about the cfe-commits
mailing list