Page MenuHomePhabricator

[clang-tidy] [readability-convert-member-functions-to-static] ignore functions that hide base class methods

Authored by mgehre on Aug 16 2019, 1:31 PM.

Event Timeline

mgehre created this revision.Aug 16 2019, 1:31 PM
Herald added a project: Restricted Project. · View Herald TranscriptAug 16 2019, 1:31 PM
Herald added a subscriber: xazax.hun. · View Herald Transcript
Eugene.Zelenko edited reviewers, added: alexfh, hokein; removed: Eugene.Zelenko.Aug 16 2019, 1:35 PM
Eugene.Zelenko added a project: Restricted Project.

I'd like some help in understanding the motivation for this change. The bug report says But the intention behind this is clearly that there is a member function A::foo which derived classes overwrite. and I'm not certain that's so clear. It's been my experience that hidden member function names are unusual and it's hard to ascribe intent to them.

FWIW, I would find this new behavior to be confusing. My mental model for this check is "if it doesn't need to be a member function, make it a static function instead", and this behavior doesn't fit that model.

I struggle myself to fully understand the motivation behind the bug report. This is certainly not code that I would write.
So I don't really care about that kind of code and I didn't want to be in the way of other people's way of writing C++.

Alternatively, we could ask the bug reporter to use // NOLINT to support his specific case. I will try to get some clarification
from the reporter.

mgehre abandoned this revision.Sep 5 2019, 2:33 PM

The discussions on the bug did not provide further insight (until now).