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

Reply via email to