alexfh (Alexander Kornienko)
User

Projects

User does not belong to any projects.

User Details

User Since
Jul 10 2012, 10:35 AM (250 w, 4 d)

Recent Activity

Yesterday

alexfh committed rL301743: [clang-tidy] Expand AllowConditional*Casts to binary logical operators.
[clang-tidy] Expand AllowConditional*Casts to binary logical operators
Sat, Apr 29, 5:20 AM

Fri, Apr 28

alexfh accepted D32395: [clang-tidy] modernize-use-emplace: remove unnecessary make_pair calls.

LG

Fri, Apr 28, 5:08 AM · Restricted Project

Wed, Apr 26

alexfh committed rL301431: [clang-tidy] Update IdentifierNamingCheck to remove extra leading/trailing….
[clang-tidy] Update IdentifierNamingCheck to remove extra leading/trailing…
Wed, Apr 26, 9:52 AM
alexfh closed D32333: [clang-tidy] Update IdentifierNamingCheck to remove extra leading/trailing underscores by committing rL301431: [clang-tidy] Update IdentifierNamingCheck to remove extra leading/trailing….
Wed, Apr 26, 9:52 AM
alexfh accepted D32436: [clang-tidy] Support detecting for-range loop in inefficient-vector-operation check..

LG

Wed, Apr 26, 9:51 AM
alexfh added a comment to D32333: [clang-tidy] Update IdentifierNamingCheck to remove extra leading/trailing underscores.

Done, thanks for the review!

What is the procedure for merging patches in? I'm sure I don't have permissions to do it myself.

Wed, Apr 26, 9:41 AM
alexfh added a comment to D32436: [clang-tidy] Support detecting for-range loop in inefficient-vector-operation check..

Cool! A few nits.

Wed, Apr 26, 6:57 AM
alexfh accepted D32333: [clang-tidy] Update IdentifierNamingCheck to remove extra leading/trailing underscores.
LG with a nit.
Wed, Apr 26, 6:37 AM

Thu, Apr 20

alexfh added a comment to D24892: [clang-tidy] Add option "LiteralInitializers" to cppcoreguidelines-pro-type-member-init.

Is there any way for multiple checks to share an option?

There's OptionsView::getLocalOrGlobal. See how StrictMode option is read in ArgumentCommentCheck, for example.

ArgumentCommentCheck uses getLocalOrGlobal, but InefficientStringConcatenationCheck and SuspiciousEnumUsageCheck don't.
6 checks have an IncludeStyle option that isn't shared.
4 checks share a HeaderFileExtensions option, but with different defaults.

Thu, Apr 20, 5:39 PM
alexfh added inline comments to D32294: [clang-tidy] run-clang-tidy.py: check if clang-apply-replacements succeeds.
Thu, Apr 20, 5:24 PM · Restricted Project
alexfh added a comment to D32294: [clang-tidy] run-clang-tidy.py: check if clang-apply-replacements succeeds.

After thinking about Piotr's comment, I think that a better way to perform the check would be te invoking clang-apply-replacements with --version and seeing if it fails even before running clang-tidy.
This way it is possible to save a lot of time when run-clang-tidy.py is run on a big project and apply_fixes fails at the end.

Let me know what you think about this approach.

Sounds good to me!

Thu, Apr 20, 5:20 PM · Restricted Project
alexfh accepted D32294: [clang-tidy] run-clang-tidy.py: check if clang-apply-replacements succeeds.

SGTM provided this continues to work with python 2.

Thu, Apr 20, 12:43 PM · Restricted Project
alexfh added a comment to D24892: [clang-tidy] Add option "LiteralInitializers" to cppcoreguidelines-pro-type-member-init.

The modernize-use-default-member-init check now has an option with the same effect, but it is called UseAssignment.
We should use consistent option names.
Is there any way for multiple checks to share an option?

Thu, Apr 20, 9:02 AM
alexfh requested changes to D32294: [clang-tidy] run-clang-tidy.py: check if clang-apply-replacements succeeds.
Thu, Apr 20, 8:59 AM · Restricted Project

Tue, Apr 18

alexfh added a comment to D32118: Cleanup some GraphTraits iteration code.

Thank you! I'll let Tim commit this patch, if he has no more concerns.

Tue, Apr 18, 4:49 PM
alexfh added inline comments to D24886: Add [[clang::suppress(rule, ...)]] attribute.
Tue, Apr 18, 11:13 AM
alexfh accepted D31542: [clang-tidy] Extend readability-container-size-empty to add comparisons to newly-constructed objects.

LG with one nit.

Tue, Apr 18, 11:03 AM · Restricted Project
alexfh committed rL300569: [clang-tidy] Fix google-explicit-constructor issue with out-of-line conversions.
[clang-tidy] Fix google-explicit-constructor issue with out-of-line conversions
Tue, Apr 18, 10:38 AM
alexfh added a comment to D31757: [clang-tidy] Add a clang-tidy check for possible inefficient vector operations.

Awesome, thanks! A few late comments inline.

Tue, Apr 18, 10:27 AM
alexfh accepted D32164: [clang-tidy] misc-misplaced-widening-cast: Disable checking of implicit widening casts by default.

LG

Tue, Apr 18, 9:56 AM
alexfh created D32170: Add a FixItHint for -Wmissing-prototypes to insert 'static '..
Tue, Apr 18, 9:17 AM
alexfh added a comment to D32118: Cleanup some GraphTraits iteration code.

LGTM for GraphTraits-related changes (thought I also see an interface change on Split, which I know nothing about).

I suppose, this change needs at least to be mentioned in the patch description along with a short explanation of why it is a no-op (I hope, it is? ;).

Tue, Apr 18, 5:21 AM
alexfh added a comment to D32118: Cleanup some GraphTraits iteration code.

LGTM for GraphTraits-related changes (thought I also see an interface change on Split, which I know nothing about).

Tue, Apr 18, 5:17 AM

Mon, Apr 17

alexfh accepted D32118: Cleanup some GraphTraits iteration code.

LG with one more comment.

Mon, Apr 17, 7:22 AM

Sun, Apr 16

alexfh added inline comments to D32118: Cleanup some GraphTraits iteration code.
Sun, Apr 16, 3:38 AM

Thu, Apr 13

alexfh added inline comments to D31860: Add more examples to clang tidy checkers.
Thu, Apr 13, 9:34 AM · Restricted Project

Tue, Apr 11

alexfh committed rL299927: [clang-format] Handle NSString literals by merging tokens..
[clang-format] Handle NSString literals by merging tokens.
Tue, Apr 11, 3:07 AM
alexfh closed D31706: [clang-format] Handle NSString literals by merging tokens. by committing rL299927: [clang-format] Handle NSString literals by merging tokens..
Tue, Apr 11, 3:07 AM
alexfh accepted D31862: docs/clang-tidy/tools/dump_check_docs.py: Remove deprecated script.

LG

Tue, Apr 11, 2:28 AM
alexfh added a comment to D31860: Add more examples to clang tidy checkers.

Another couple of post-commit comments.

Tue, Apr 11, 2:13 AM · Restricted Project
alexfh added a comment to rL299920: Add more examples to clang tidy checkers.

Another couple of late comments.

Tue, Apr 11, 2:10 AM

Mon, Apr 10

alexfh accepted D31860: Add more examples to clang tidy checkers.

LG with a couple of nits.

Mon, Apr 10, 12:58 PM · Restricted Project

Fri, Apr 7

alexfh added a comment to D31706: [clang-format] Handle NSString literals by merging tokens..

Friendly ping.

Fri, Apr 7, 5:47 AM
alexfh committed rL299752: [clang-tidy] A couple of minor fixes in modernize-use-using tests.
[clang-tidy] A couple of minor fixes in modernize-use-using tests
Fri, Apr 7, 2:54 AM

Thu, Apr 6

alexfh added a comment to D30930: [clang-tidy-vs] Visual Studio plugin extended.

Re: binary files. Are you using git? Then, I think, you need to do something similar to https://reviews.llvm.org/D31763: add a .gitattributes file containing "*.png binary" and use arcanist to upload the patch (not sure if Phab's web-interface can allow uploading patches with binary files). I also don't know whether git-svn will handle "svn propset svn:mime-type application/octet-stream ...", but that's a problem of whoever will submit the patch for you (Zachary, probably, since it makes sense to at least build the new code and I don't have Windows and MSVS).

Thu, Apr 6, 8:00 AM
alexfh updated the diff for D31763: TEST git+phab+binary files.

Remove irrelevant files.

Thu, Apr 6, 7:53 AM
alexfh created D31763: TEST git+phab+binary files.
Thu, Apr 6, 7:48 AM
alexfh committed rL299651: [clang-tidy] Update docs and help message.
[clang-tidy] Update docs and help message
Thu, Apr 6, 7:39 AM
alexfh committed rL299649: [clang-tidy] Add FormatStyle configuration option..
[clang-tidy] Add FormatStyle configuration option.
Thu, Apr 6, 6:54 AM
alexfh accepted D31713: [Basic] getColumnNumber returns location of CR+LF on Windows.

LG with one nit.

Thu, Apr 6, 2:12 AM

Wed, Apr 5

alexfh created D31706: [clang-format] Handle NSString literals by merging tokens..
Wed, Apr 5, 6:35 AM
alexfh committed rL299535: Fix WebAssembly after r299529..
Fix WebAssembly after r299529.
Wed, Apr 5, 5:03 AM

Tue, Apr 4

alexfh added a comment to D31648: [clang-tidy] new performance-expensive-value-param check.

Very nice analysis and documentation! I wonder only whether it's better to make this a part of the performance-unnecessary-value-param check? I'm also not sure why automatic fixes are any more difficult than that of performance-unnecessary-value-param?

Tue, Apr 4, 6:41 AM
alexfh added inline comments to D31308: [clang-tidy] new check readability-no-alternative-tokens.
Tue, Apr 4, 5:57 AM

Mar 30 2017

alexfh accepted D31406: [clang-tidy] Reuse FileID in getLocation.

LG
Do you have commit rights?

Mar 30 2017, 2:55 PM
alexfh added inline comments to D31308: [clang-tidy] new check readability-no-alternative-tokens.
Mar 30 2017, 9:28 AM
alexfh requested changes to D31406: [clang-tidy] Reuse FileID in getLocation.

Thank you for finding out the root cause here!

Mar 30 2017, 9:19 AM
alexfh updated subscribers of D31406: [clang-tidy] Reuse FileID in getLocation.
Mar 30 2017, 9:07 AM

Mar 27 2017

alexfh added inline comments to D31308: [clang-tidy] new check readability-no-alternative-tokens.
Mar 27 2017, 9:51 AM

Mar 24 2017

alexfh added a comment to D31308: [clang-tidy] new check readability-no-alternative-tokens.

FWIW, I'm pretty sure this can and should be done on the lexer level - it will be faster and more universal.

Mar 24 2017, 4:33 AM
alexfh added a comment to D31308: [clang-tidy] new check readability-no-alternative-tokens.

So I would propose to keep the features as-is for now,
change the name to readability-operators-representation, and then later (someone else?) might also add an option
for making this work the other way around. Would that be ok for you?

Mar 24 2017, 4:31 AM
alexfh added a comment to D31308: [clang-tidy] new check readability-no-alternative-tokens.

Thanks for your feedback, Eugene!

I'm not sure if it would be helpful to have this check in both ways. I did a code search for "not_eq", "bitand" and "and_eq"
on github, and their usage seems to be a clear minority.

So I would propose to keep the features as-is for now,
change the name to readability-operators-representation, and then later (someone else?) might also add an option
for making this work the other way around. Would that be ok for you?

Were you refering to https://reviews.llvm.org/D25195 ("[lit] Fix FormatError on individual test timeout")?

Mar 24 2017, 4:28 AM
alexfh added a comment to D29262: Fixes to modernize-use-using.

I have plan to finish this patch next week, when I finish academic year at my school. If I will have any issues with submitting, Prazek offered to help me.

Mar 24 2017, 4:08 AM

Mar 23 2017

alexfh committed rL298619: [clang-tidy] Don't use groups in the big regexy filter.
[clang-tidy] Don't use groups in the big regexy filter
Mar 23 2017, 9:42 AM
alexfh added a comment to D29262: Fixes to modernize-use-using.

Krystyna, do you need help committing the patch after you address the outstanding comments?

Mar 23 2017, 8:33 AM
alexfh committed rL298608: [clang-tidy] Fix diag message for catch-by-value.
[clang-tidy] Fix diag message for catch-by-value
Mar 23 2017, 8:30 AM
alexfh closed D30592: [clang-tidy] Fix diag message for catch-by-value by committing rL298608: [clang-tidy] Fix diag message for catch-by-value.
Mar 23 2017, 8:30 AM
alexfh added a comment to D29858: [clang-tidy] Catch trivially true statements like a != 1 || a != 3.

Following up. Was this checked in? Do I need to do anything further?

Mar 23 2017, 8:28 AM
alexfh committed rL298607: [clang-tidy] Catch trivially true statements like a != 1 || a != 3.
[clang-tidy] Catch trivially true statements like a != 1 || a != 3
Mar 23 2017, 8:26 AM
alexfh closed D29858: [clang-tidy] Catch trivially true statements like a != 1 || a != 3 by committing rL298607: [clang-tidy] Catch trivially true statements like a != 1 || a != 3.
Mar 23 2017, 8:26 AM
alexfh accepted D30567: [clang-tidy] Fix treating non-space whitespaces in checks list..

Do you have commit rights?

Mar 23 2017, 8:25 AM
alexfh removed reviewers for D15797: [clang-tidy] Fix readability-braces-around-statements assert failure: aaron.ballman, alexfh.
Mar 23 2017, 7:39 AM
alexfh requested changes to D31252: [clang-tidy] add readability-compound-statement-size check..

Hi Roman,

Mar 23 2017, 7:28 AM · Restricted Project

Mar 22 2017

alexfh added a comment to D27210: [clang-tidy] misc-string-compare. Adding a new check to clang-tidy.

A late comment here: the check seems to suit better readability module instead of misc. Could you move it there?

Mar 22 2017, 9:38 AM
alexfh added inline comments to D30567: [clang-tidy] Fix treating non-space whitespaces in checks list..
Mar 22 2017, 7:53 AM
alexfh added a comment to D30567: [clang-tidy] Fix treating non-space whitespaces in checks list..

There's one more trim() you missed. And the test needs to be updated (s/\\n/ /).

Mar 22 2017, 7:53 AM
alexfh added a comment to D31130: B32239 clang-tidy should not warn about array to pointer decay on system macros.

This change was reverted in r298470. The use of the <assert.h> include is a problem because this is not a clang-supplied header file. Also, there's a (good) question about what array decay is happening in the assert(false) test case.

Mar 22 2017, 6:04 AM
alexfh committed rL298500: [clang-tidy] clang-format the last patch. NFC.
[clang-tidy] clang-format the last patch. NFC
Mar 22 2017, 6:02 AM
alexfh committed rL298501: [clang-tidy] Tests should not rely on STL headers being available..
[clang-tidy] Tests should not rely on STL headers being available.
Mar 22 2017, 6:02 AM
alexfh committed rL298499: [clang-tidy] modified identifier naming case to use CT_AnyCase for ignoring….
[clang-tidy] modified identifier naming case to use CT_AnyCase for ignoring…
Mar 22 2017, 6:02 AM
alexfh closed D30931: [clang-tidy] modified identifier naming case to use CT_AnyCase for ignoring case style by committing rL298499: [clang-tidy] modified identifier naming case to use CT_AnyCase for ignoring….
Mar 22 2017, 6:02 AM
alexfh added a comment to D30567: [clang-tidy] Fix treating non-space whitespaces in checks list..

Hi Alex and sorry for the late reply.

The main use case is a more readable .clang-tidy configuration checks.
Before this correction one can use something like this:

---
Checks: '
    ,*,
    ,-cert-dcl03-c,
'
...

It works, but is hardly comprehensible to a newbie (the strange use of addtional commas).
Since the spaces are ignored (since a recent commit of yours) we can add spaces after the leading comma and hope that no user uses a tab...

After applying this patch, we can just write (with tabs or spaces and as many newlines as we want - used for grouping for instance):

---
Checks: '
    *,
    -cert-dcl03-c,
'
...

Additionaly, you can sometimes accidentally issue a tabulator on the command line and that's just nice to ignore it.

Mar 22 2017, 3:38 AM
alexfh accepted D30931: [clang-tidy] modified identifier naming case to use CT_AnyCase for ignoring case style.

A couple of nits, otherwise looks good. Do you need me to commit the patch for you?

Mar 22 2017, 3:11 AM

Mar 20 2017

alexfh created D31160: [clang-tidy] Fix misc-move-const-arg for move-only types..
Mar 20 2017, 4:33 PM
alexfh committed rL298316: [clang-tidy] readability-container-size-empty fix for (*x).size().
[clang-tidy] readability-container-size-empty fix for (*x).size()
Mar 20 2017, 3:27 PM
alexfh committed rL298315: [clang-tidy] Small cleanup. NFC..
[clang-tidy] Small cleanup. NFC.
Mar 20 2017, 3:27 PM

Mar 18 2017

alexfh added a comment to D31097: [clang-tidy] don't warn about implicit widening casts in function calls.

I wonder whether warning on implicit casts still makes sense for example in mission critical code. So maybe it is worth to have a configuration option with the default setting being less strict and chatty. What do you think?

But it's not about "misplaced casts", it's about implicit conversions and -Wconversion diagnostic can take care of this.

Mar 18 2017, 2:47 AM
alexfh added a comment to D31097: [clang-tidy] don't warn about implicit widening casts in function calls.

I wonder whether warning on implicit casts still makes sense for example in mission critical code. So maybe it is worth to have a configuration option with the default setting being less strict and chatty. What do you think?

Mar 18 2017, 2:38 AM

Mar 17 2017

alexfh added a comment to D30931: [clang-tidy] modified identifier naming case to use CT_AnyCase for ignoring case style.

Could you generate a diff with full context (http://llvm.org/docs/Phabricator.html#requesting-a-review-via-the-web-interface)?

Mar 17 2017, 10:33 AM
alexfh committed rL298101: [Clang-tidy] Fix for misc-noexcept-move-constructor false triggers on defaulted….
[Clang-tidy] Fix for misc-noexcept-move-constructor false triggers on defaulted…
Mar 17 2017, 9:52 AM
alexfh closed D31049: [Clang-tidy] Fix for misc-noexcept-move-constructor false triggers on defaulted declarations by committing rL298101: [Clang-tidy] Fix for misc-noexcept-move-constructor false triggers on defaulted….
Mar 17 2017, 9:52 AM · Restricted Project
alexfh added a comment to D31049: [Clang-tidy] Fix for misc-noexcept-move-constructor false triggers on defaulted declarations.

Do you have commit rights?

Mar 17 2017, 6:07 AM · Restricted Project
alexfh accepted D31049: [Clang-tidy] Fix for misc-noexcept-move-constructor false triggers on defaulted declarations.

LG

Mar 17 2017, 6:06 AM · Restricted Project
alexfh committed rL298060: [clang-tidy] Added a test with a different format..
[clang-tidy] Added a test with a different format.
Mar 17 2017, 3:17 AM
alexfh committed rL298059: [clang-tidy] readability-misleading-indentation: fix chained if.
[clang-tidy] readability-misleading-indentation: fix chained if
Mar 17 2017, 3:10 AM
alexfh closed D30841: [clang-tidy] readability-misleading-indentation: fix chained if by committing rL298059: [clang-tidy] readability-misleading-indentation: fix chained if.
Mar 17 2017, 3:10 AM
alexfh committed rL298057: [clang-tidy] Verify some conditions in a matcher instead of check(). NFC.
[clang-tidy] Verify some conditions in a matcher instead of check(). NFC
Mar 17 2017, 2:59 AM
alexfh accepted D30841: [clang-tidy] readability-misleading-indentation: fix chained if.

Now using ASTContext::getParents instead of ChainedIfs map.

For some reason I thought of getParents as an expensive function to call...

Mar 17 2017, 2:50 AM
alexfh committed rL298052: [clang-tidy] Ignore deleted members in google-explicit-constructor..
[clang-tidy] Ignore deleted members in google-explicit-constructor.
Mar 17 2017, 1:52 AM

Mar 16 2017

alexfh added inline comments to D30841: [clang-tidy] readability-misleading-indentation: fix chained if.
Mar 16 2017, 7:42 AM
alexfh requested changes to D30841: [clang-tidy] readability-misleading-indentation: fix chained if.

Thank you for the fix! One comment inline.

Mar 16 2017, 7:36 AM
alexfh requested changes to D30931: [clang-tidy] modified identifier naming case to use CT_AnyCase for ignoring case style.
Mar 16 2017, 7:13 AM
alexfh requested changes to D30748: [Lexer] Finding beginning of token with escaped new line.
Mar 16 2017, 6:00 AM
alexfh added a comment to D30931: [clang-tidy] modified identifier naming case to use CT_AnyCase for ignoring case style.

You are right, it does look misleading. I'll try to modify it the way you suggest (though I'm new to LLVM, so be ready to give me more comments;)

Mar 16 2017, 2:37 AM

Mar 14 2017

alexfh added a comment to D30931: [clang-tidy] modified identifier naming case to use CT_AnyCase for ignoring case style.

BTW, next time please add cfe-commits to subscribers when you create the patch to get it sent properly to the mailing list.

Mar 14 2017, 9:49 AM
alexfh updated subscribers of D30931: [clang-tidy] modified identifier naming case to use CT_AnyCase for ignoring case style.
Mar 14 2017, 9:48 AM
alexfh edited reviewers for D30931: [clang-tidy] modified identifier naming case to use CT_AnyCase for ignoring case style, added: berenm; removed: rsmith.
Mar 14 2017, 9:47 AM
alexfh added a comment to D30931: [clang-tidy] modified identifier naming case to use CT_AnyCase for ignoring case style.

I understand your use case, but this patch makes the check's behavior more confusing: having both "any case" and "ignore case" with subtle differences in behavior seems very misleading. The problem seems to be coming from the usage of CT_AnyCase to denote uninitialized style. Should we just remove NamingStyle::isSet and use llvm::Optional<NamingStyle> instead of NamingStyle where appropriate?

Mar 14 2017, 9:46 AM

Mar 12 2017

alexfh added inline comments to D30610: [clang-tidy] Added options to cppcoreguidelines-special-member-functions check.
Mar 12 2017, 4:11 PM

Mar 11 2017

alexfh added a reviewer for D30748: [Lexer] Finding beginning of token with escaped new line: rsmith.
Mar 11 2017, 6:54 AM