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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
