pytypes.h: constrain accessor::operator= templates so that they do not obscure special members#5832
Conversation
…t match calls that should use the special member functions. Found by an experimental, new clang-tidy check. While we may not know the exact design decisions now, it seems unlikely that the special members were deliberately meant to not be selected (for otherwise they could have been defined differently to make this clear). Rather, it seems like an oversight that the operator templates win in overload resolution, and we should restore the intended resolution.
|
(To be squashed on commit.) |
rwgk
left a comment
rwgk
left a comment
There was a problem hiding this comment.
Thanks @tkoeppe!
This bug is super unobvious! (I'm very surprised that the overload resolution is so unintuitive in this case.)
I'll merge this now since all tests pass. If you can think of a unit test that catches this, it'd be ideal to add it in.
|
Thanks, Ralf! Yes, that's why we have this new clang-tidy check, I suppose! It'll hopefully be released before long. I'm also running a global test, I'll let you know if anything weird comes up. |
Great to know! Are all the sanitizer tests still in place? (I don't think we have any sanitizer coverage here anymore.) |
|
Yes, the pybind.*san tests are all still in place and have passed. |
|
@rwgk: And global testing is all good, too. |
Awesome, thanks! |
2 participants
Description
Constraints the templates
accessor::operator=so that they are not selected in overload resolution when the special members should be selected.Found by an experimental, new clang-tidy check.
While we may not know the exact design decisions now, it seems unlikely that the special members were deliberately meant to not be selected (for otherwise they could have been defined differently to make this clear). Rather, it seems like an oversight that the operator templates win in overload resolution, and we should restore the intended resolution.
Should not have any observable behaviour.