Fix a clang warning [-Wshadow-field-in-constructor-modified]

[asmaloney]

Description

Fix warning from Apple clang version 11.0.0 (clang-1100.0.33.17)

Suggested changelog entry:

* fix warning modifying constructor parameter 'flag' that shadows a field of 'set_flag' [-Wshadow-field-in-constructor-modified]

[YannickJadoul]

my_flag sounds very tutorial-for-programming-like, though. Could you change this to flag_ or f or so (or suggest something else)?

Also (mostly to other reviewers): should we turn on more warning flags in CI? What's our goal when it comes to such warnings?

[asmaloney]

I figured you would have a preferred naming for something like this, I just couldn't find it. I use m for member style (mFlag) so I never have to deal with this in my own code.

I'll use flag_.

[YannickJadoul]

Thanks! I can't really find it either. We do use m_flag for private class members, but this is a struct, so flag is part of the interface (well, it's a local struct in a function, so it's not that important, but you don't want to write some_struct.m_flag, I think :-) )

[YannickJadoul]

A quick look seems that f and flag_ would be the two options. I found instances of both conventions. I think this is fine, though, so thanks!

[YannickJadoul]

Can't hurt to silence this, I suppose.

[YannickJadoul]

I'm surprised this is the only one you find, though? I can spot a bunch of other struct members and constructor arguments in attr.h. But those don't fire that warning for you?

[asmaloney]

I'm surprised this is the only one you find, though?

Hmmm... you're right - that is surprising.

I'm using a tiny subset of the API, but I see that struct doc and struct name have the same issue.

[henryiii]

This is a pretty specific warning, due to the fact you modify a value passed in. This is really weird out of context, it's setting the value and then changing the input to True?

bool val = False;
set_flag new_flag(val);
// val is now True, new_flag contains False.

Big fan of the change making this clearer!

[YannickJadoul]

Oh, I think I see what's going on. Isn't this guarding against re-entrance/infinite recursion of the implicit conversion, during the PyObject *result = PyObject_Call((PyObject *) type, args.ptr(), nullptr); call?

[YannickJadoul]

This is a pretty specific warning, due to the fact you modify a value passed in. This is really weird out of context, it's setting the value and then changing the input to True?

Right, I see, indeed. This could be an issue because it's not clear which flag = true; (should be the parameter you're setting?). In this case, it doesn't really matter though, since both are references, so it doesn't matter which one is changed. At any rate, it's not wrong either, to fix it, so let's just merge it and remove a warning :-)