[clang-tidy] fix duplicate '{}' in cppcoreguidelines-pro-type-member-init

[Sockke]

Summary

The overload of the constructor will repeatedly fix the member variables that need to be initialized.
Removed the duplicate '{}'.

struct A {
  A() {}
  A(int) {}
  int _var;  // int _var{}{};  <--  wrong fix
};

[MTC]

clang-tools-extra/clang-tidy/cppcoreguidelines/ProTypeMemberInitCheck.cpp

36

Btw, the parameter Record seems useless, could you clean it up in another patch?

439

I believe DenseSet::contains is more appropriate here.

453

FieldsToInit being empty implies AllFieldsToInit is empty. We have checked the emptiness of FieldsToInit.

clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-pro-type-member-init.cpp

210–211

Please update the comments.

[MTC]

clang-tools-extra/clang-tidy/cppcoreguidelines/ProTypeMemberInitCheck.cpp

453

never mind, what I said is wrong！

[Sockke]

Format and add some description.

[Sockke]

Any thoughts?

[Sockke]

Any thoughts? : )

[aaron.ballman]

clang-tools-extra/clang-tidy/cppcoreguidelines/ProTypeMemberInitCheck.cpp

439

+1 to the suggestion to use contains().

clang-tools-extra/clang-tidy/cppcoreguidelines/ProTypeMemberInitCheck.h

79

[Sockke]

update! @aaron.ballman @MTC

[aaron.ballman]

LGTM, thank you!

clang-tools-extra/clang-tidy/cppcoreguidelines/ProTypeMemberInitCheck.h

 //===--- ProTypeMemberInitCheck.h - clang-tidy-------------------*- C++ -*-===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
 #ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_CPPCOREGUIDELINES_PRO_TYPE_MEMBER_INIT_H
 #define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_CPPCOREGUIDELINES_PRO_TYPE_MEMBER_INIT_H
 
 #include "../ClangTidyCheck.h"
+#include "llvm/ADT/DenseSet.h"
 
 namespace clang {
 namespace tidy {
 namespace cppcoreguidelines {
 
 /// Implements C++ Core Guidelines Type.6.
 ///
 /// Checks that every user-provided constructor value-initializes all class
private:
 
   // Whether arrays need to be initialized or not. Default is false.
   bool IgnoreArrays;
 
   // Whether fix-its for initialization of fundamental type use assignment
   // instead of brace initialization. Only effective in C++11 mode. Default is
   // false.
   bool UseAssignment;
+
+  // Record the member variables that have been initialized to prevent repeated
+  // initialization.
+  llvm::DenseSet<const FieldDecl *> HasRecordClassMemberSet;
 };
 
 } // namespace cppcoreguidelines
 } // namespace tidy
 } // namespace clang
 
 #endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_CPPCOREGUIDELINES_PRO_TYPE_MEMBER_INIT_H

clang-tools-extra/clang-tidy/cppcoreguidelines/ProTypeMemberInitCheck.cpp

 
 AST_MATCHER(CXXRecordDecl, hasDefaultConstructor) {
   return Node.hasDefaultConstructor();
 }
 
 // Iterate over all the fields in a record type, both direct and indirect (e.g.
 // if the record contains an anonymous struct).
 template <typename T, typename Func>
 void forEachField(const RecordDecl &Record, const T &Fields, Func &&Fn) {
   for (const FieldDecl *F : Fields) {
     if (F->isAnonymousStructOrUnion()) {
       if (const CXXRecordDecl *R = F->getType()->getAsCXXRecordDecl())
         forEachField(*R, R->fields(), Fn);
     } else {
       Fn(F);
     }
   }
void ProTypeMemberInitCheck::checkMissingMemberInitializer(
 
   // Collect all fields in order, both direct fields and indirect fields from
   // anonymous record types.
   SmallVector<const FieldDecl *, 16> OrderedFields;
   forEachField(ClassDecl, ClassDecl.fields(),
                [&](const FieldDecl *F) { OrderedFields.push_back(F); });
 
   // Collect all the fields we need to initialize, including indirect fields.
+  // It only includes fields that have not been fixed
   SmallPtrSet<const FieldDecl *, 16> AllFieldsToInit;
-  forEachField(ClassDecl, FieldsToInit,
-               [&](const FieldDecl *F) { AllFieldsToInit.insert(F); });
-  if (AllFieldsToInit.empty())
+  forEachField(ClassDecl, FieldsToInit, [&](const FieldDecl *F) {
+    if (!HasRecordClassMemberSet.contains(F)) {
+      AllFieldsToInit.insert(F);
+      HasRecordClassMemberSet.insert(F);
+    }
+  });
+  if (FieldsToInit.empty())
     return;
 
   DiagnosticBuilder Diag =
       diag(Ctor ? Ctor->getBeginLoc() : ClassDecl.getLocation(),
            "%select{|union }0constructor %select{does not|should}0 initialize "
            "%select{|one of }0these fields: %1")
-      << IsUnion << toCommaSeparatedString(OrderedFields, AllFieldsToInit);
+      << IsUnion << toCommaSeparatedString(OrderedFields, FieldsToInit);
+
+  if (AllFieldsToInit.empty())
+    return;
 
   // Do not propose fixes for constructors in macros since we cannot place them
   // correctly.
   if (Ctor && Ctor->getBeginLoc().isMacroID())
     return;
 
   // Collect all fields but only suggest a fix for the first member of unions,
   // as initializing more than one union member is an error.

clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-pro-type-member-init.cpp

struct PositiveMultipleConstructors {
   PositiveMultipleConstructors() {}
   // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: constructor does not initialize these fields: A, B
 
   PositiveMultipleConstructors(int) {}
   // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: constructor does not initialize these fields: A, B
 
   PositiveMultipleConstructors(const PositiveMultipleConstructors &) {}
   // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: constructor does not initialize these fields: A, B
 
-  // FIXME: The fix-its here collide providing an erroneous fix
   int A, B;
-  // CHECK-FIXES: int A{}{}{}, B{}{}{};
+  // CHECK-FIXES: int A{}, B{};
 };
 
 typedef struct {
   int Member;
 } CStyleStruct;
 
 struct PositiveUninitializedBase : public NegativeAggregateType, CStyleStruct {
   PositiveUninitializedBase() {}