[clang-tidy] Fix handling of UseAssignment option in cppcoreguidelines-prefer-member-initializer

[PiotrZSL]

Summary

From now on check will use value from cppcoreguidelines-prefer-member-initializer
and fallback to modernize-use-default-member-init.UseAssignment if not specified.

Fixes: #55616.

[carlosgalvezp]

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

134

This is very strange, feels like it's done to ensure the checks are in sync but IMO it creates more harm than good and makes the check harder to maintain. The checks are independent anyway (not aliases), so I believe it makes sense to keep being independent also in the options.

There are similar checks (e.g. magic numbers) where the user needs to either only enable one of the checks, or enter the same configuration settings twice.

I would vote for just treating this like an independent argument like all other checks, to avoid bugs like these.

[PiotrZSL]

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

134

So remove dependency on modernize-use-default-member-init completly, or just on modernize-use-default-member-init options ?

[carlosgalvezp]

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

134

I had a look at the original patch:

https://reviews.llvm.org/D71199

There's quite some discussion about the topic, and reviewers like @aaron.ballman weren't very happy about introducing this coupling. Seems like the problem is non-trivial so I vote for leaving your patch as is, which fixes a Github ticket and brings value, and think about completely removing the dependency to modernize-use-default-member-init on a separate patch.

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

isAssignmentToMemberOf(const CXXRecordDecl *Rec, const Stmt *S,
   return std::make_pair(nullptr, nullptr);
 }
 
 PreferMemberInitializerCheck::PreferMemberInitializerCheck(
     StringRef Name, ClangTidyContext *Context)
     : ClangTidyCheck(Name, Context),
       IsUseDefaultMemberInitEnabled(
           Context->isCheckEnabled("modernize-use-default-member-init")),
-      UseAssignment(OptionsView("modernize-use-default-member-init",
+      UseAssignment(
+          Options.get("UseAssignment",
+                      OptionsView("modernize-use-default-member-init",
                                   Context->getOptions().CheckOptions, Context)
-                        .get("UseAssignment", false)) {}
+                          .get("UseAssignment", false))) {}
 
 void PreferMemberInitializerCheck::storeOptions(
     ClangTidyOptions::OptionMap &Opts) {
   Options.store(Opts, "UseAssignment", UseAssignment);
 }
 
 void PreferMemberInitializerCheck::registerMatchers(MatchFinder *Finder) {
   Finder->addMatcher(

clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines/prefer-member-initializer.rst

.. code-block:: c++
   C(int nn, int mm) : n(nn) {
     if (dice())
       return;
     m = mm;
   }
 
 .. option:: UseAssignment
 
-   If this option is set to `true` (default is `false`), the check will initialize
-   members with an assignment. In this case the fix of the first example looks
-   like this:
+   If this option is set to `true` (by default `UseAssignment` from
+   `modernize-use-default-member-init
+   <../modernize/use-default-member-init.html>`_ will be used),
+   the check will initialize members with an assignment.
+   In this case the fix of the first example looks like this:
 
 .. code-block:: c++
 
   class C {
     int n = 1;
     int m;
   public:
     C() {
       if (dice())
         return;
       m = 1;
     }
   };

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

 // RUN: %check_clang_tidy %s cppcoreguidelines-prefer-member-initializer,modernize-use-default-member-init %t -- \
 // RUN: -config="{CheckOptions: [{key: modernize-use-default-member-init.UseAssignment, value: true}]}"
+// RUN: %check_clang_tidy %s cppcoreguidelines-prefer-member-initializer,modernize-use-default-member-init %t -- \
+// RUN: -config="{CheckOptions: [{key: modernize-use-default-member-init.UseAssignment, value: false}, \
+// RUN:                          {key: cppcoreguidelines-prefer-member-initializer.UseAssignment, value: true}]}"
 
 class Simple1 {
   int n;
   // CHECK-FIXES: int n = 0;
   double x;
   // CHECK-FIXES: double x = 0.0;
 
 public:

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

 // RUN: %check_clang_tidy %s cppcoreguidelines-prefer-member-initializer,modernize-use-default-member-init %t
+// RUN: %check_clang_tidy %s cppcoreguidelines-prefer-member-initializer,modernize-use-default-member-init %t -- \
+// RUN: -config="{CheckOptions: [{key: modernize-use-default-member-init.UseAssignment, value: true}, \
+// RUN:                          {key: cppcoreguidelines-prefer-member-initializer.UseAssignment, value: false}]}"
 
 class Simple1 {
   int n;
   // CHECK-FIXES: int n{0};
   double x;
   // CHECK-FIXES: double x{0.0};
 
 public: