Making C function declaration parameter names consistent with corresponding definition names
От | Peter Geoghegan |
---|---|
Тема | Making C function declaration parameter names consistent with corresponding definition names |
Дата | |
Msg-id | CAH2-WznJt9CMM9KJTMjJh_zbL5hD9oX44qdJ4aqZtjFi-zA3Tg@mail.gmail.com обсуждение исходный текст |
Ответы |
Re: Making C function declaration parameter names consistent with corresponding definition names
|
Список | pgsql-hackers |
I applied clang-tidy's readability-inconsistent-declaration-parameter-name check with "readability-inconsistent-declaration-parameter-name.Strict:true" [1] to write the attached refactoring patch series. The patch series makes parameter names consistent between each function's definition and declaration. The check made the whole process of getting everything to match straightforward. The total number of lines changed worked out at less than you might guess it would, since we mostly tend to do this already: 178 files changed, 593 insertions(+), 582 deletions(-) I have to admit that these inconsistencies are a pet peeve of mine. I find them distracting, and have a history of fixing them on an ad-hoc basis. But there are real practical arguments in favor of being strict about it as a matter of policy -- it's not *just* neatnikism. First there is a non-zero potential for bugs by allowing inconsistencies. Consider the example of the function check_usermap(), from hba.c. The bool argument named "case_insensitive" is inverted in the declaration, where it is spelled "case_sensitive". At first I thought that this might be a security bug, and reported it to -security as such. It's harmless, but is still arguably something that might have led to a real bug. Then there is the "automated refactoring" argument. It would be nice to make automated refactoring tools work a little better by always (or almost always) having a clean slate to start with. In general refactoring work might involve writing a patch that starts with the declarations that appear in a some .h file of interest in one pass, and work backwards from there. It might be necessary to switch dozens of functions over to some new naming convention or parameter order, so you really want to start with the high level interface in such a scenario. It's rather nice to be able to use clang-tidy to make sure that there are no newly introduced inconsistencies -- which have the potential to be live bugs. It's possible to use clang-tidy for this process right now, but it's not as easy as it could be because you have to ignore any preexisting minor inconsistencies. We don't quite have a clean slate to start from, which makes it more error prone. IMV there is a lot to be said for making this a largely mechanical process, with built in guard rails. Why not lean on the tooling that's widely available already? Introducing a project policy around consistent parameter names would make this easy. I believe that it would be practical and unobtrusive -- we almost do this already, without any policy in place (it only took me a few hours to come up with the patch series). I don't think that we need to create new work for committers to do this. [1] https://releases.llvm.org/14.0.0/tools/clang/tools/extra/docs/clang-tidy/checks/readability-inconsistent-declaration-parameter-name.html -- Peter Geoghegan
Вложения
В списке pgsql-hackers по дате отправления: