Page MenuHomePhabricator

alexfh (Alexander Kornienko)
User

Projects

User does not belong to any projects.

User Details

User Since
Jul 10 2012, 10:35 AM (431 w, 6 d)

Recent Activity

Thu, Oct 15

alexfh committed rGcc175c2cc8e6: Support ObjC in IncludeInserter (authored by alexfh).
Support ObjC in IncludeInserter
Thu, Oct 15, 7:15 PM
alexfh closed D89276: Support ObjC in IncludeInserter.
Thu, Oct 15, 7:15 PM · Restricted Project, Restricted Project, Restricted Project
alexfh accepted D89276: Support ObjC in IncludeInserter.

LG

Thu, Oct 15, 11:46 AM · Restricted Project, Restricted Project, Restricted Project

Wed, Oct 14

alexfh requested changes to D89380: [clang-tidy] Fix for cppcoreguidelines-prefer-member-initializer to handle classes declared in macros.
Wed, Oct 14, 7:08 AM · Restricted Project, Restricted Project
alexfh added a comment to D89380: [clang-tidy] Fix for cppcoreguidelines-prefer-member-initializer to handle classes declared in macros.

Thanks for the fix! However, I'm not sure it's possible to correctly rewrite code in all cases where macros are involved. See a couple of motivating examples in the comment.

Wed, Oct 14, 7:06 AM · Restricted Project, Restricted Project

Tue, Oct 13

alexfh added reviewers for D89276: Support ObjC in IncludeInserter: ymandel, gribozavr2.
Tue, Oct 13, 5:58 AM · Restricted Project, Restricted Project, Restricted Project

Mon, Oct 12

alexfh abandoned D2660: Store warning option for custom diagnostic messages..
Mon, Oct 12, 7:26 AM
alexfh abandoned D5602: [clang-tidy] Add support for boolean check options..
Mon, Oct 12, 7:08 AM
alexfh abandoned D5830: [clang-tidy] Alternative approach to OptionsView::get.
Mon, Oct 12, 7:07 AM
alexfh abandoned D6580: Add a way to tell MatchFinder not to visit template instantiations and implicit code..
Mon, Oct 12, 6:59 AM
alexfh abandoned D31763: TEST git+phab+binary files.
Mon, Oct 12, 6:58 AM
alexfh requested changes to D55346: [clang-tidy] check for using declaration qualification.
Mon, Oct 12, 6:57 AM · Restricted Project
alexfh requested changes to D48866: [clang-tidy] Add incorrect-pointer-cast checker.
Mon, Oct 12, 6:57 AM · Restricted Project
alexfh requested changes to D42682: [clang-tidy] Add io-functions-misused checker.
Mon, Oct 12, 6:56 AM · Restricted Project
alexfh requested changes to D33841: [clang-tidy] redundant 'extern' keyword check.
Mon, Oct 12, 6:55 AM · Restricted Project
alexfh requested changes to D49864: [clang-tidy] The script clang-tidy-diff.py doesn't accept 'pass by' options (--).
Mon, Oct 12, 6:42 AM · Restricted Project, Restricted Project
alexfh added a comment to D49864: [clang-tidy] The script clang-tidy-diff.py doesn't accept 'pass by' options (--).

Sorry for the delay. This patch fell through the cracks. If you're still interested, could you rebase it on top of current HEAD and upload a full diff? Or use the arcanist tool, see https://llvm.org/docs/Phabricator.html.

Mon, Oct 12, 6:41 AM · Restricted Project, Restricted Project
alexfh accepted D82089: [clang-tidy] modernize-loop-convert reverse iteration support.

Looks good!

Mon, Oct 12, 6:36 AM · Restricted Project
alexfh added a comment to D79674: [clang-tidy] Better support for Override function in RenamerClangTidy based checks.

Feel free to ping patches every week or so. It looks like in this case all the reviewers were swamped with something else at the time.

Mon, Oct 12, 6:32 AM · Restricted Project
alexfh accepted D79674: [clang-tidy] Better support for Override function in RenamerClangTidy based checks.

Looks good! Thanks for the fix! IIUC, this is related to https://bugs.llvm.org/show_bug.cgi?id=34879? Makes sense to specify this in the patch description.

Mon, Oct 12, 6:31 AM · Restricted Project
alexfh committed rG1968a6155fd5: [clang-tidy] Fix IncludeInserter usage example in a comment. (authored by alexfh).
[clang-tidy] Fix IncludeInserter usage example in a comment.
Mon, Oct 12, 6:06 AM

Fri, Oct 9

alexfh committed rGfe4715c47f9c: Remove old create(MainFile)?IncludeInsertion overloads (authored by alexfh).
Remove old create(MainFile)?IncludeInsertion overloads
Fri, Oct 9, 6:25 AM
alexfh closed D89117: Remove old create(MainFile)?IncludeInsertion overloads.
Fri, Oct 9, 6:25 AM · Restricted Project
alexfh requested review of D89117: Remove old create(MainFile)?IncludeInsertion overloads.
Fri, Oct 9, 4:50 AM · Restricted Project

Mon, Sep 28

alexfh committed rGfdfe324da195: [clang-tidy] IncludeInserter: allow <> in header name (authored by alexfh).
[clang-tidy] IncludeInserter: allow <> in header name
Mon, Sep 28, 6:14 AM
alexfh closed D85666: [clang-tidy] IncludeInserter: allow <> in header name.
Mon, Sep 28, 6:14 AM · Restricted Project
alexfh updated the diff for D85666: [clang-tidy] IncludeInserter: allow <> in header name.
  • Updated release notes and default option values in docs
Mon, Sep 28, 5:59 AM · Restricted Project
alexfh added inline comments to D85666: [clang-tidy] IncludeInserter: allow <> in header name.
Mon, Sep 28, 5:18 AM · Restricted Project
alexfh updated the diff for D85666: [clang-tidy] IncludeInserter: allow <> in header name.
  • clang-format
  • addressed review comments
Mon, Sep 28, 5:18 AM · Restricted Project

Sep 14 2020

alexfh accepted D83144: Allow to specify macro names for android-comparison-in-temp-failure-retry..

Looks good modulo comment.

Sep 14 2020, 1:15 AM · Restricted Project

Aug 10 2020

alexfh added inline comments to D82089: [clang-tidy] modernize-loop-convert reverse iteration support.
Aug 10 2020, 10:24 AM · Restricted Project
alexfh requested review of D85666: [clang-tidy] IncludeInserter: allow <> in header name.
Aug 10 2020, 10:23 AM · Restricted Project
alexfh removed a reviewer for D81254: [analyzer] Produce symbolic values for C-array elements: alexfh.
Aug 10 2020, 8:48 AM · Restricted Project
alexfh accepted D85218: In clang-tidy base checks prevent anonymous functions from triggering assertions.

LG with a couple of comments. Do you need someone to land the patch for you?

Aug 10 2020, 8:45 AM · Restricted Project, Restricted Project
alexfh requested changes to D82089: [clang-tidy] modernize-loop-convert reverse iteration support.

Thanks for the patch! Looks generally good. A few comments inline.

Aug 10 2020, 6:29 AM · Restricted Project

Jul 21 2020

alexfh added a comment to D83223: [clang-tidy] Header guard check can skip past license comment.
 // This is not identified as a license comment as the
// block is followed by code.
void foo();

FWIW: https://github.com/GrammaTech/gtirb-pprinter/blob/master/include/gtirb_pprinter/AttPrettyPrinter.hpp or https://github.com/GrammaTech/gtirb/blob/master/include/gtirb/AuxData.hpp (so there are projects which do not put a newline between the license and code).

Short of creating an AI that understands context it won't be possible to determine the difference between license and general documentation, in any case I feel this heuristic is the safest way to ensure good coverage with minimised risk of inserting the guard in the middle of documentation,

My instinct is that we shouldn't be trying to play those games in the first place and should consider *all* leading comments and empty (whitespace-only) lines as part of the "license" and expect the first significant token to be the header guard. e.g., this isn't about the license at all, it's about whether you can have prose before the header guard or not. It's not uncommon for projects to put prose before header guards, nor is it uncommon for it to go after the header guards. tbh, that feels a bit like an option for the feature rather than an automatic behavior because I could also see a project wanting to enforce a consistent style.

Jul 21 2020, 2:15 AM · Restricted Project

Apr 13 2020

alexfh added inline comments to D77983: clang-tidy doc: add a note for every checker with an autofix.
Apr 13 2020, 5:52 AM · Restricted Project
alexfh accepted D75184: [clang-tidy] Optional inheritance of file configs from parent directories .

Apologies for the delay! It's sort of a crazy time now =\

Apr 13 2020, 5:20 AM · Restricted Project, Restricted Project

Apr 12 2020

alexfh committed rG8dda0f919959: Remove dependency between test files. (authored by alexfh).
Remove dependency between test files.
Apr 12 2020, 9:23 PM

Mar 19 2020

alexfh added a comment to D75184: [clang-tidy] Optional inheritance of file configs from parent directories .

You are absolutely right about current behaviour. Thank you for catching this odd behaviour. I'm not 100% confident what is the right behaviour but my guess is that overriding local option from parent config with global from child folder is better so I implemented it and added corresponding test.

It's a tricky one that, but I thing overriding the local option with the global in the sub directory is the correct way to go about this as well.

Mar 19 2020, 9:22 PM · Restricted Project, Restricted Project

Mar 12 2020

alexfh added inline comments to D75911: [clang-tidy] Added hasAnyListedName matcher.
Mar 12 2020, 4:51 PM · Restricted Project, Restricted Project
alexfh added inline comments to D76083: [clang-tidy] Expand the list of functions in bugprone-unused-return-value.
Mar 12 2020, 3:12 PM · Restricted Project

Mar 5 2020

alexfh added inline comments to D75538: [clang-tidy] Updated language supported restrictions on some checks.
Mar 5 2020, 4:56 AM · Restricted Project

Mar 4 2020

alexfh added inline comments to D75538: [clang-tidy] Updated language supported restrictions on some checks.
Mar 4 2020, 6:11 AM · Restricted Project
alexfh requested changes to D75184: [clang-tidy] Optional inheritance of file configs from parent directories .

Internally we have something similar, but with unconditional inheritance and a way to include other configs. I was thinking about implementing this in the FileOptionsProvider, but decided that this would be an overkill for most real projects. Overall the patch looks good to me, but please update the documentation and add a test to ensure that check options get merged correctly.

Mar 4 2020, 5:39 AM · Restricted Project, Restricted Project

Mar 3 2020

alexfh requested changes to D75538: [clang-tidy] Updated language supported restrictions on some checks.
Mar 3 2020, 9:56 AM · Restricted Project
alexfh added a comment to D75441: [clang-tidy] Add helper base check classes that only run on specific language versions.

I think my preference is to go with isLanguageVersionSupported() and not use base classes.

+1, I can see this working, but the introduction of new concepts does not seem like it pulls its weight given the tiny amount of boilerplate that they eliminate.

Mar 3 2020, 3:04 AM · Restricted Project

Mar 2 2020

alexfh committed rG071002ffdb3f: [clang-tidy] Copy the Ranges field from the Diagnostic when creating the… (authored by compositeprimes).
[clang-tidy] Copy the Ranges field from the Diagnostic when creating the…
Mar 2 2020, 3:54 AM
alexfh closed D68887: [clang-tidy] Copy the Ranges field from the Diagnostic when creating the ClangTidyError.
Mar 2 2020, 3:54 AM · Restricted Project, Restricted Project

Feb 27 2020

alexfh accepted D68887: [clang-tidy] Copy the Ranges field from the Diagnostic when creating the ClangTidyError.

LG

Feb 27 2020, 11:35 AM · Restricted Project, Restricted Project
alexfh added a comment to D69782: Summary: Instead of dropping all the ranges associated with a Diagnostic when converting them to a ClangTidy error, instead attach them to the ClangTidyError, so they can be consumed by other APIs..

Next step is to plumb this to clang-tidy.

Feb 27 2020, 11:34 AM · Restricted Project
alexfh added a comment to D69782: Summary: Instead of dropping all the ranges associated with a Diagnostic when converting them to a ClangTidy error, instead attach them to the ClangTidyError, so they can be consumed by other APIs..

I don't have commit rights (this is my first llvm commit!), so could someone help out? Thanks!

Feb 27 2020, 10:41 AM · Restricted Project
alexfh committed rGb26c88e3c6e0: [clang-tidy] Store all ranges in clang::tooling::Diagnostic (authored by compositeprimes).
[clang-tidy] Store all ranges in clang::tooling::Diagnostic
Feb 27 2020, 10:41 AM
alexfh closed D69782: Summary: Instead of dropping all the ranges associated with a Diagnostic when converting them to a ClangTidy error, instead attach them to the ClangTidyError, so they can be consumed by other APIs..
Feb 27 2020, 10:40 AM · Restricted Project

Feb 24 2020

alexfh accepted D69746: [analyzer] FixItHint: Apply and test hints with the Clang-Tidy's script.

LG

Feb 24 2020, 5:31 PM · Restricted Project

Feb 21 2020

alexfh accepted D69782: Summary: Instead of dropping all the ranges associated with a Diagnostic when converting them to a ClangTidy error, instead attach them to the ClangTidyError, so they can be consumed by other APIs..

Do you have commit rights or should someone help you committing the patch?

Feb 21 2020, 9:37 AM · Restricted Project

Feb 19 2020

alexfh added a comment to D72730: [clang-tidy] run-clang-tidy -export-fixes exports only fixes, not all warnings.

On second thought maybe this should be fixed in clang-tidy and not here?
Maybe besides -export-fixes there should be an -export-warnings or just -yaml-export?

Feb 19 2020, 5:24 AM · Restricted Project, Restricted Project
alexfh added a comment to D69746: [analyzer] FixItHint: Apply and test hints with the Clang-Tidy's script.

A few nits.

Feb 19 2020, 5:20 AM · Restricted Project
alexfh added inline comments to D69782: Summary: Instead of dropping all the ranges associated with a Diagnostic when converting them to a ClangTidy error, instead attach them to the ClangTidyError, so they can be consumed by other APIs..
Feb 19 2020, 3:28 AM · Restricted Project

Feb 18 2020

alexfh added a comment to D69782: Summary: Instead of dropping all the ranges associated with a Diagnostic when converting them to a ClangTidy error, instead attach them to the ClangTidyError, so they can be consumed by other APIs..

Thanks for the update!

Feb 18 2020, 1:55 AM · Restricted Project

Feb 1 2020

alexfh added inline comments to D73580: [clang-tidy] rename_check.py: maintain alphabetical order in Renamed checks section.
Feb 1 2020, 6:40 PM · Restricted Project, Restricted Project
alexfh accepted D73841: [clang-tidy] Fixed crash 44745 in readability-else-after-return.

LG. Thanks for the fix!

Feb 1 2020, 6:23 PM · Restricted Project, Restricted Project

Jan 28 2020

alexfh added a comment to D72527: [clang-tidy] adjust scripts to subsubsections in Release Notes.

As I wrote, I don't have GitHub commit access, so I need help with commit.

Jan 28 2020, 3:46 AM · Restricted Project, Restricted Project

Jan 27 2020

alexfh accepted D72527: [clang-tidy] adjust scripts to subsubsections in Release Notes.

Just ping.

Jan 27 2020, 4:46 PM · Restricted Project, Restricted Project
alexfh added a comment to D73464: [clang] Add TagDecl AST matcher.

A drive-by comment.

Jan 27 2020, 2:16 PM · Restricted Project
alexfh added a comment to D73413: [clang-tidy] Add check to detect external definitions with no header declaration.

How is this different from -Wmissing-prototypes?

Jan 27 2020, 2:07 PM · Restricted Project, Restricted Project

Jan 23 2020

alexfh accepted D73236: [clang-tidy] Add clang-tidy headers to clang distribution.

LG with an outstanding comment.

Jan 23 2020, 3:43 PM · Restricted Project, Restricted Project

Jan 21 2020

alexfh added a reviewer for D72242: Fix crash on value dependent bitfields in if conditions in templates: gribozavr2.
Jan 21 2020, 1:35 AM · Restricted Project

Jan 13 2020

alexfh added a comment to D72527: [clang-tidy] adjust scripts to subsubsections in Release Notes.

Both scripts works fine. However rename script should also sort entries alphabetically, but probably this should be separate patch.

Jan 13 2020, 10:25 AM · Restricted Project, Restricted Project
alexfh added a comment to D72527: [clang-tidy] adjust scripts to subsubsections in Release Notes.

LG with a nit, if this works.

Jan 13 2020, 6:07 AM · Restricted Project, Restricted Project

Jan 9 2020

alexfh accepted D72421: [clang-tidy] Refresh the add_new_check.py now that we use a table + autofix.

Awesome! Thanks for updating the script! A few nits, otherwise LG, if this works.

Jan 9 2020, 7:43 AM · Restricted Project, Restricted Project

Jan 8 2020

alexfh added a comment to D71594: testing clang-tidy.

What version of clang-tidy and clang-format does this run? Whatever is available in Debian packages?

Jan 8 2020, 8:28 AM · Restricted Project, Restricted Project

Jan 3 2020

alexfh added a comment to D36051: Move from a long list of checkers to tables.

This list used to be automatically updated by the clang-tools-extra/clang-tidy/add_new_check.py script. I suppose, this is broken now?

Jan 3 2020, 11:50 AM · Restricted Project

Dec 12 2019

alexfh committed rG65996c302a44: [clang-tidy] Use early returns to make the code easier to read and potentially… (authored by alexfh).
[clang-tidy] Use early returns to make the code easier to read and potentially…
Dec 12 2019, 8:05 AM
alexfh committed rG2b09390c1362: Fix naming style. NFC. (authored by alexfh).
Fix naming style. NFC.
Dec 12 2019, 8:05 AM

Dec 10 2019

alexfh requested changes to D69782: Summary: Instead of dropping all the ranges associated with a Diagnostic when converting them to a ClangTidy error, instead attach them to the ClangTidyError, so they can be consumed by other APIs..
Dec 10 2019, 5:43 AM · Restricted Project

Dec 6 2019

alexfh committed rGfac4e3c5f8a0: [clang-tidy] Fix PR26274 (authored by alexfh).
[clang-tidy] Fix PR26274
Dec 6 2019, 1:13 AM
alexfh closed D70974: [clang-tidy] Fix PR26274.
Dec 6 2019, 1:13 AM · Restricted Project

Dec 5 2019

alexfh added inline comments to D70974: [clang-tidy] Fix PR26274.
Dec 5 2019, 3:16 AM · Restricted Project
alexfh updated the diff for D70974: [clang-tidy] Fix PR26274.
  • Added a test with an empty attribute list.
Dec 5 2019, 3:16 AM · Restricted Project

Dec 4 2019

alexfh added a comment to D70974: [clang-tidy] Fix PR26274.

I'm not convinced this feature is worth implementing at all, because there's a good alternative to a macro here -- a namespace alias. What is the reason to use a macro instead of a namespace alias?

While I think that's a superior solution to using macros, some users have macros instead. This fixes a bug reported in https://bugs.llvm.org/show_bug.cgi?id=26274 and I agree that the behavior described in that bug is not what I would expect it to be.

I mean, it is possible to break pretty much any ClangTidy check with sneaky code. But there's a limit to which we should try to make things work in tricky corner cases. For example, the fix in this patch does not handle function-like macros (namespace MY_LIBRARY_NAMESPACE_FOR_VERSION(42) {).

LGTM if we must... but I don't think we should.

Dec 4 2019, 9:32 AM · Restricted Project
alexfh updated the diff for D70974: [clang-tidy] Fix PR26274.
  • Added support for inline namespaces and namespace attributes, fixed a typo, added tests.
Dec 4 2019, 8:17 AM · Restricted Project

Dec 3 2019

alexfh committed rGc375dc230d16: Revert "Fix llvm-namespace-comment for macro expansions" (authored by alexfh).
Revert "Fix llvm-namespace-comment for macro expansions"
Dec 3 2019, 11:38 AM
alexfh added a reverting change for rG4736d63f752f: Fix llvm-namespace-comment for macro expansions: rGc375dc230d16: Revert "Fix llvm-namespace-comment for macro expansions".
Dec 3 2019, 11:38 AM
alexfh created D70974: [clang-tidy] Fix PR26274.
Dec 3 2019, 11:38 AM · Restricted Project
alexfh added a comment to D69855: [clang-tidy] Fix llvm-namespace-comment for macro expansions.

There's a problem with this patch. Consider the following code:

// In some remote header:
#define SOME_RANDOM_MACRO internal
Dec 3 2019, 7:27 AM · Restricted Project, Restricted Project

Nov 28 2019

alexfh added inline comments to D68682: format::cleanupAroundReplacements removes whole line when Removals leave previously non-blank line blank.
Nov 28 2019, 8:17 AM · Restricted Project, Restricted Project

Nov 27 2019

alexfh added a comment to D69746: [analyzer] FixItHint: Apply and test hints with the Clang-Tidy's script.

The only way I know people apply fixits is with the help of IDEs.

Nov 27 2019, 11:23 AM · Restricted Project
alexfh accepted D46027: [clang-tidy] Fix PR35824.

This approach will also introduce false negatives.

Nov 27 2019, 10:45 AM · Restricted Project

Nov 26 2019

alexfh committed rGaa0e92e1f706: [clang-tidy] Use range-for for check registration. NFC (authored by alexfh).
[clang-tidy] Use range-for for check registration. NFC
Nov 26 2019, 7:48 AM

Nov 14 2019

alexfh added a comment to D70165: clang-tidy: modernize-use-override new option AllowOverrideAndFinal.

While I have no objections against this patch, I wonder whether someone had a chance to ask GCC developers about this? Is it a conscious choice to suggest override when final is present? What's the argument for doing so?

Nov 14 2019, 5:02 AM · Restricted Project

Oct 18 2019

alexfh added inline comments to D68885: Add a Ranges field to Diagnostic struct.
Oct 18 2019, 7:13 AM · Restricted Project, Restricted Project

Oct 16 2019

alexfh accepted D69036: [libTooling] Fix r374962: add more Transformer forwarding decls..

LG.

Oct 16 2019, 7:21 AM · Restricted Project

Oct 14 2019

alexfh added a comment to D68682: format::cleanupAroundReplacements removes whole line when Removals leave previously non-blank line blank.

Thanks, from the name that sounds like the perfect place to do it. If cleanupAroundReplacements also is used by clang-format, would we want to make the functionality optional, e.g. via a new bool parameter to cleanupAroundReplacements, a new option in FormatStyle, etc.?

Oct 14 2019, 1:16 AM · Restricted Project, Restricted Project

Oct 11 2019

alexfh accepted D68807: [ClangTidy] Separate tests for infrastructure and checkers.

I agree that the test/clang-tidy directory has become hard to navigate. Splitting tests for checks and for infrastructure seems reasonable to me. I personally don't care about specific directory names and file moves/renames are also not a concern, since git is rather good at handling those. I personally strongly prefer long-term improvements over desire to minimize churn.

Oct 11 2019, 4:51 AM · Restricted Project

Sep 26 2019

alexfh edited reviewers for D67865: [clang-tidy] Finds uses of OSRead* calls on macOS that may mask unexpected behavior due to unaligned reads, added: gribozavr; removed: alexfh.
Sep 26 2019, 2:07 AM · Restricted Project

Sep 23 2019

alexfh added a comment to D67501: [clang-tidy] Fix relative path in header-filter..

Sorry, I reverted this patch in r372601.

Unfortunately, it makes paths printed in clang-tidy'd diagnostics inconsistent with what -header-filter operates on.

For example, imagine that file-filter.cpp includes header_alias.h, which is a symlink to header.h. The diagnostics printed by clang-tidy refer to header_alias.h, however, after this patch, -header-filter logic calls realpath() and operates on header.h -- making it very difficult for users to understand how exactly to set up filters.

Also, note that paths in diagnostics don't collapse foo/.. or symlinks.

We should only change both sides of path handling simultaneously (paths in diagnostics and paths in filters). However, since getting the user's preferred path is potentially very difficult when symlinks are present, I'm not sure if anything can be done here. Maybe only collapsing foo/.. would be viable?

Sep 23 2019, 6:51 AM · Restricted Project, Restricted Project, Restricted Project

Sep 18 2019

alexfh edited reviewers for D67567: [clang-tidy] New check to warn when storing dispatch_once_t in non-static, non-global storage, added: gribozavr; removed: alexfh.
Sep 18 2019, 6:30 AM · Restricted Project, Restricted Project, Restricted Project

Sep 17 2019

alexfh added a comment to D45050: [clang-tidy] New checker for not null-terminated result caused by strlen(), size() or equal length.

A couple of drive-by comments.

Sep 17 2019, 6:42 AM · Restricted Project, Restricted Project
alexfh removed a reviewer for D65917: [clang-tidy] Added check for the Google style guide's category method naming rule.: alexfh.
Sep 17 2019, 5:32 AM · Restricted Project, Restricted Project