[clang-tidy] Fix crash in `modernize-use-default-member-init`

[AMS21]

Summary

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

[PiotrZSL]

clang-tools-extra/clang-tidy/modernize/UseDefaultMemberInitCheck.cpp

274–276

instead of ignoring array types, better would be just not provide FixItHints for them, so WRN would stil be emited.

[AMS21]

Still issue a warning just no fixit

[PiotrZSL]

Maybe we should still support arrays.... in some way

clang-tools-extra/clang-tidy/modernize/UseDefaultMemberInitCheck.cpp

273

const bool ValueInit = isa<ImplicitValueInitExpr>(InitExpression) && !isa<ArrayType>(InitType);

This should do a trick.

[AMS21]

Should now properly handle arrays

[Dinistro]

Thanks for fixing this issue.

[PiotrZSL]

LGTM

[AMS21]

Great, would someone be so kind and push this on by behalf :)

clang-tools-extra/clang-tidy/modernize/UseDefaultMemberInitCheck.cpp

auto InitBase =
             floatLiteral(),
             unaryOperator(hasAnyOperatorName("+", "-"),
                           hasUnaryOperand(floatLiteral())),
             cxxBoolLiteral(), cxxNullPtrLiteralExpr(), implicitValueInitExpr(),
             declRefExpr(to(enumConstantDecl())));
 
   auto Init =
       anyOf(initListExpr(anyOf(allOf(initCountIs(1), hasInit(0, InitBase)),
-                               initCountIs(0))),
+                               initCountIs(0), hasType(arrayType()))),
             InitBase);
 
   Finder->addMatcher(
       cxxConstructorDecl(forEachConstructorInitializer(
           cxxCtorInitializer(
               forField(unless(anyOf(
                   getLangOpts().CPlusPlus20 ? unless(anything()) : isBitField(),
                   hasInClassInitializer(anything()),
void UseDefaultMemberInitCheck::checkDefaultInit(
   SourceLocation FieldEnd =
       Lexer::getLocForEndOfToken(Field->getSourceRange().getEnd(), 0,
                                  *Result.SourceManager, getLangOpts());
   SourceLocation LParenEnd = Lexer::getLocForEndOfToken(
       Init->getLParenLoc(), 0, *Result.SourceManager, getLangOpts());
   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());
 }
 
 void UseDefaultMemberInitCheck::checkExistingInit(
     const MatchFinder::MatchResult &Result, const CXXCtorInitializer *Init) {
   const FieldDecl *Field = Init->getAnyMember();
 
   if (!sameValue(Field->getInClassInitializer(), Init->getInit()))
     return;
 
   diag(Init->getSourceLocation(), "member initializer for %0 is redundant")
-      << Field
-      << FixItHint::CreateRemoval(Init->getSourceRange());
+      << Field << FixItHint::CreateRemoval(Init->getSourceRange());
 }
 
 } // namespace clang::tidy::modernize

clang-tools-extra/docs/ReleaseNotes.rst

- Improved :doc:`modernize-concat-nested-namespaces
   using macro between namespace declarations, to fix false positive when using namespace
   with attributes and to support nested inline namespace introduced in c++20.
 
 - In :doc:`modernize-use-default-member-init
   <clang-tidy/checks/modernize/use-default-member-init>` count template
   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
   member functions in unions.
 
 - Improved :doc:`modernize-use-override
   <clang-tidy/checks/modernize/use-override>` check with new
   `IgnoreTemplateInstantiations` option to optionally ignore virtual function

clang-tools-extra/test/clang-tidy/checkers/modernize/use-default-member-init-assignment.cpp

 template <typename T>
 struct NegativeTemplate {
     NegativeTemplate() : t() {}
     T t;
 };
 
 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

clang-tools-extra/test/clang-tidy/checkers/modernize/use-default-member-init.cpp

 
 struct EmptyBracedIntDefault {
   EmptyBracedIntDefault() : m_i{} {}
   int m_i;
   // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: use default member initializer for 'm_i' [modernize-use-default-member-init]
   // 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