aaron.ballman added inline comments.
================ Comment at: clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-prefer-member-initializer-assignment.cpp:29 +public: + Simple2() : n (0) { + x = 0.0; ---------------- baloghadamsoftware wrote: > aaron.ballman wrote: > > By my reading of the core guideline > > (https://github.com/isocpp/CppCoreGuidelines/blob/master/CppCoreGuidelines.md#c48-prefer-in-class-initializers-to-member-initializers-in-constructors-for-constant-initializers), > > it looks like `n` should also be diagnosed because all of the constructors > > in the class initialize the member to the same constant value. Is there a > > reason to deviate from the rule (or have I missed something)? > > > > Also, I'd like to see a test case like: > > ``` > > class C { > > int n; > > public: > > C() { n = 0; } > > explicit C(int) { n = 12; } > > }; > > ``` > This check only cares for initializations inside the body (rule `C.49`, but > if the proper fix is to convert them to default member initializer according > to rule `C.48` then we follow that rule in the fix). For initializations > implemented as constructor member initializers but according to `C.48` they > should have been implemented as default member initializers we already have > check `modernize-use-default-member-init`. Thank you for the explanation. I have the feeling that these two rules are going to have some really weird interactions together. For instance, the example you added at my request shows behavior I can't easily explain as a user: ``` class Complex19 { int n; // CHECK-FIXES: int n{0}; public: Complex19() { n = 0; // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: 'n' should be initialized in an in-class default member initializer [cppcoreguidelines-prefer-member-initializer] // CHECK-FIXES: {{^\ *$}} } explicit Complex19(int) { // CHECK-FIXES: Complex19(int) : n(12) { n = 12; // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: 'n' should be initialized in a member initializer of the constructor [cppcoreguidelines-prefer-member-initializer] // CHECK-FIXES: {{^\ *$}} } ~Complex19() = default; }; ``` Despite both constructors having the assignment expression `n = <literal>;` one gets one diagnostic + fixit and the other gets a different diagnostic + fixit. Also, from reading C.49, I don't see anything about using in-class initialization. I see C.49 as being about avoiding the situation where the ctor runs a constructor for a data member only to then immediately in the ctor body make an assignment to that member. You don't need to initialize then reinitialize when you can use a member init list to construct the object properly initially. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D71199/new/ https://reviews.llvm.org/D71199 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits