@alexfh: Ah, I forgot about this review. I will mark it as abandoned, because I have already started work on new check for localizing variables. It will have the wider scope that Eugene proposed originally, that is to move variable declarations closer to their first use. If you're curious how the code looks, you can see it on my Github fork: https://github.com/piotrdz/clang-tools-extra, as I said in discussion on cfe-dev some time ago. I will submit this new check in a new review once I finish it.

@eugene: thanks for clarifying that and your advice. I made the commit and it seems I didn't break anything, yet :-).

I have obtained now commit rights but I want to make sure: I can commit this patch now that it is accepted? That is, accepting the review is the same as saying "LGTM", right?

Thanks for accepting the review. However, I still have to ask someone to make the commit. Or should I obtain commit rights myself? If so, how do I do that?

Any other comments?

Addressed last issues in review

There you go, third revision. I think this version should be ready for commit.

Addressed latest review issues.

I think this is a more complete version, and we're getting closer to getting my patch committed. @sbenza: Do you agree?

I fixed the issues found in review and added a few other enhancements:

• add config options to allow integer and pointer casts in conditionals, e.g. "if (pointer)",
• add support for NULL macro cast to boolean,
• ignore usage of variable declaration in control statements, e.g. "if (int x = function())",
• add support case of false literal to pointer conversion in pre-C++11 standards
• propose NULL instead of nullptr in replacements in pre-C++11 standards

@sbenza: Thanks for the review. I have added my comments with expanations as necessary, and I have already fixed some issues you reported. I'm not posting a new diff at this time, as I want to finish with remaining issues, and then post updated diff once I'm ready with everything. I'll also work on adding a config option to conditionally allow for pointer to bool casts, as discussed on cfe-dev mailing list.

Ah, no, I don't - so far I have relied on other people to help with commits. I'd appreciate it if you could commit this for me. Thanks.

@aaron.ballman: Thanks for the review. I uploaded an updated diff with fixes.

Addressed issues found in review

\o/ Yay! Thanks!

@alexfh: here we go again. Any comments?

Again, addressed all review issues. I hope this is the final version

@alexfh: What do you think now? Are we getting nearer to making a commit?

I hope this is the final re-write of my code. In this version, I addressed most recent review comments, while also refactoring code to better handle template specializations and correctly display diagnostics in case when function definition is visible. In the end I had to change over half of the code, but perhaps it was worth it.

Now that I fixed all review issues, I think this version would be acceptable for commit. @alexfh: do you agree?
There are of course two outstanding issues marked with TODO and FIXME comments, but these are areas of improvement. I would like to first discuss the best solution to them, before attempting to fix them in code.

I addressed all latest review issues.

In this third version I did the following:

• fixed problems which I noticed with template specializations,
• changed output diagnostics to be more detailed, covering multiple declarations
• added FixIt hints to refactor inconsistent declarations to parameter names we see in definition
• added new template-related testcases and extended existing ones with checking of note diagnostics and FixIt hints

Now this issue with templates is a bit difficult for me. I tried everything that seems to have made sense, but I still get output like this:

/work/clang-trunk/clang-tools-extra/test/clang-tidy/readability-inconsistent-declaration-parameter-name.cpp:47:6: warning: function 'templateFunction' has other declaration with different parameter name [readability-inconsistent-declaration-parameter-name]
void templateFunction(float b)
     ^
/work/clang-trunk/clang-tools-extra/test/clang-tidy/readability-inconsistent-declaration-parameter-name.cpp:47:6: note: other declaration seen here
void templateFunction(float b)
     ^
/work/clang-trunk/clang-tools-extra/test/clang-tidy/readability-inconsistent-declaration-parameter-name.cpp:47:29: note: parameter 1 is named 'b' here, but 'a' in other declaration
void templateFunction(float b)
                            ^

Applied fixes for most issues found in review.

@alexfh: Thanks for reviewing my code. The amount of comments makes it pretty clear that it's my first contribution here :-). But no worries, I'll address each issue.

@eugene: I don't understand, what does declaring function with "void" argument have in common with this review? I only check here variable declarations inside functions.

I noticed a few things I should have done better:

• use std::set instead of std::unordered_set (it doesn't seem to be used elsewhere)
• move checking set of reported functions to just before emitting diagnostic
• improved documentation
• changed diff directory to inside clang-tools-extra repository