Page MenuHomePhabricator

piotrdz (Piotr Dziwinski)
User

Projects

User does not belong to any projects.

User Details

User Since
Aug 28 2015, 5:49 PM (362 w, 3 d)

Recent Activity

Nov 6 2015

piotrdz abandoned D12473: [clang-tidy] Add old style function check.

@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.

Nov 6 2015, 3:05 PM

Oct 25 2015

piotrdz committed rL251244: [clang-tidy] Another fix for failing buildbots regarding signedness of char.
[clang-tidy] Another fix for failing buildbots regarding signedness of char
Oct 25 2015, 10:13 AM
piotrdz committed rL251239: [clang-tidy] Fix for build bots not liking #include <cstddef>.
[clang-tidy] Fix for build bots not liking #include <cstddef>
Oct 25 2015, 8:49 AM
piotrdz added a comment to D13635: [clang-tidy] Add readability-implicit-bool-cast check.

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

Oct 25 2015, 8:38 AM
piotrdz committed rL251235: [clang-tidy] Add check readability-implicit-bool-cast.
[clang-tidy] Add check readability-implicit-bool-cast
Oct 25 2015, 8:33 AM
piotrdz closed D13635: [clang-tidy] Add readability-implicit-bool-cast check by committing rL251235: [clang-tidy] Add check readability-implicit-bool-cast.
Oct 25 2015, 8:33 AM
piotrdz added a comment to D13635: [clang-tidy] Add readability-implicit-bool-cast check.

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?

Oct 25 2015, 1:41 AM

Oct 24 2015

piotrdz committed rL251204: Test commit.
Test commit
Oct 24 2015, 1:13 PM
piotrdz added a comment to D13635: [clang-tidy] Add readability-implicit-bool-cast check.

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?

Oct 24 2015, 4:34 AM

Oct 21 2015

piotrdz added a comment to D13635: [clang-tidy] Add readability-implicit-bool-cast check.

Any other comments?

Oct 21 2015, 1:36 PM
piotrdz updated the diff for D13635: [clang-tidy] Add readability-implicit-bool-cast check.

Addressed last issues in review

Oct 21 2015, 1:35 PM

Oct 20 2015

piotrdz added a comment to D13635: [clang-tidy] Add readability-implicit-bool-cast check.

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

Oct 20 2015, 3:18 PM
piotrdz updated the diff for D13635: [clang-tidy] Add readability-implicit-bool-cast check.

Addressed latest review issues.

Oct 20 2015, 3:15 PM

Oct 18 2015

piotrdz added a comment to D13635: [clang-tidy] Add readability-implicit-bool-cast check.

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

Oct 18 2015, 7:00 AM
piotrdz updated the diff for D13635: [clang-tidy] Add readability-implicit-bool-cast check.

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
Oct 18 2015, 6:54 AM

Oct 15 2015

piotrdz added a comment to D13635: [clang-tidy] Add readability-implicit-bool-cast check.

@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.

Oct 15 2015, 4:31 PM

Oct 13 2015

piotrdz added a comment to D13634: [clang-tidy] Update documentation of readability-inconsistent-declaration-parameter-names.

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.

Oct 13 2015, 11:12 AM
piotrdz added a comment to D13634: [clang-tidy] Update documentation of readability-inconsistent-declaration-parameter-names.

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

Oct 13 2015, 11:10 AM
piotrdz updated the diff for D13634: [clang-tidy] Update documentation of readability-inconsistent-declaration-parameter-names.

Addressed issues found in review

Oct 13 2015, 11:07 AM

Oct 11 2015

piotrdz retitled D13635: [clang-tidy] Add readability-implicit-bool-cast check from to [clang-tidy] Add readability-implicit-bool-cast check.
Oct 11 2015, 4:51 AM
piotrdz retitled D13634: [clang-tidy] Update documentation of readability-inconsistent-declaration-parameter-names from to [clang-tidy] Update documentation of readability-inconsistent-declaration-parameter-names.
Oct 11 2015, 4:43 AM

Sep 9 2015

piotrdz added a comment to D12462: [PATCH] [clang-tidy] Add inconsistent declaration parameter name check.

\o/ Yay! Thanks!

Sep 9 2015, 11:25 PM
piotrdz added a comment to D12462: [PATCH] [clang-tidy] Add inconsistent declaration parameter name check.

@alexfh: here we go again. Any comments?

Sep 9 2015, 12:28 PM
piotrdz updated the diff for D12462: [PATCH] [clang-tidy] Add inconsistent declaration parameter name check.

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

Sep 9 2015, 12:27 PM

Sep 8 2015

piotrdz added a comment to D12462: [PATCH] [clang-tidy] Add inconsistent declaration parameter name check.

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

Sep 8 2015, 2:54 PM
piotrdz updated the diff for D12462: [PATCH] [clang-tidy] Add inconsistent declaration parameter name check.

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.

Sep 8 2015, 2:52 PM

Sep 5 2015

piotrdz added a comment to D12462: [PATCH] [clang-tidy] Add inconsistent declaration parameter name check.

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.

Sep 5 2015, 3:40 PM
piotrdz updated the diff for D12462: [PATCH] [clang-tidy] Add inconsistent declaration parameter name check.

I addressed all latest review issues.

Sep 5 2015, 3:29 PM

Sep 2 2015

piotrdz updated the diff for D12462: [PATCH] [clang-tidy] Add inconsistent declaration parameter name check.

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
Sep 2 2015, 4:37 PM

Aug 31 2015

piotrdz added a comment to D12462: [PATCH] [clang-tidy] Add inconsistent declaration parameter name check.

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)
                            ^
Aug 31 2015, 1:16 PM
piotrdz updated the diff for D12462: [PATCH] [clang-tidy] Add inconsistent declaration parameter name check.

Applied fixes for most issues found in review.

Aug 31 2015, 1:12 PM
piotrdz added inline comments to D12462: [PATCH] [clang-tidy] Add inconsistent declaration parameter name check.
Aug 31 2015, 1:11 PM
piotrdz added a comment to D12462: [PATCH] [clang-tidy] Add inconsistent declaration parameter name check.

@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.

Aug 31 2015, 10:20 AM

Aug 30 2015

piotrdz added a comment to D12473: [clang-tidy] Add old style function check.

@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.

Aug 30 2015, 1:39 PM
piotrdz added a reviewer for D12473: [clang-tidy] Add old style function check: alexfh.
Aug 30 2015, 9:06 AM
piotrdz retitled D12473: [clang-tidy] Add old style function check from to [clang-tidy] Add old style function check.
Aug 30 2015, 3:45 AM
piotrdz updated the diff for D12462: [PATCH] [clang-tidy] Add inconsistent declaration parameter name check.

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
Aug 30 2015, 3:41 AM

Aug 28 2015

piotrdz retitled D12462: [PATCH] [clang-tidy] Add inconsistent declaration parameter name check from to [PATCH] [clang-tidy] Add inconsistent declaration parameter name check.
Aug 28 2015, 6:09 PM