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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
