[PATCH] D18271: Avoid -Wshadow warnings about constructor parameters named after fields

[Alexander Kornienko via cfe-commits]

alexfh added a comment.

In http://reviews.llvm.org/D18271#382082, @rnk wrote:

> In http://reviews.llvm.org/D18271#381769, @rsmith wrote:
>
> > I think a reasonable approach would be: "do not warn on shadowing if the idiom of naming a constructor parameter after a class member is used, and the class member is initialized from that constructor parameter,
>


Depending on how strictly you define "initialized" here, this is either unhelpful or difficult to chpatterneck. Consider following examples:

  class C1 {
    T field;
  public:
    void set_field(T f) {
      field = f;
      // do something else...
    }
    C1(T field) { set_field(field); }
  };
  
  class C2 {
    T1 f1;
    T2 f2;
  public:
    C2(T1 f1, T2 f2) {
      // Some form of initialization of `this->field` from `field` that is not just a copy construction/copy/move,
      // e.g. a method call or an assignment of a single member:
      this->f1.a = f1.a;
      this->f2.CopyFrom(f2);
    }
  };

I've seen these patterns being used in the real code, and they don't seem to be dangerous or otherwise worth being flagged by this diagnostic.

> > and all uses of the parameter in the constructor body would still be valid if it were declared `const`".

> 


That might also be overly restrictive. Consider a variant of set_field above that takes a `T&` (which is weird, but it doesn't seem like -Wshadow is the right warning in this case).

> That sounds like the right check, but I don't have a lot of time to work on this and I don't want to let the perfect be the enemy of the good. Do you think is a reasonable half-way point for us to leave it for the next year or so?

> 

> > In particular, I think we should probably warn if a non-const reference is bound to the parameter, or if the constructor's member initializer list does not contain `field(field)`.

> 


See above. There are many cases, when at least the latter pattern is used in a reasonable way.


http://reviews.llvm.org/D18271