Page MenuHomePhabricator

alexfh (Alexander Kornienko)
User

Projects

User does not belong to any projects.

User Details

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

Recent Activity

Dec 14 2020

alexfh requested changes to D91303: [clang-tidy] readability-container-size-empty: simplify implementation.

Pre-merge builder can't apply this patch: https://buildkite.com/llvm-project/diff-checks/builds/18651
Is it based on https://reviews.llvm.org/D91302 ? Do we need the intermediate state? Maybe squash the two patches together for simplicity?

Dec 14 2020, 5:29 PM · Restricted Project
alexfh requested changes to D91302: Handle template instantiations better in clang-tidy check.

Please fix the typo that results in a compile error.

Dec 14 2020, 5:24 PM · Restricted Project
alexfh committed rG9c49b0bba0fc: Remove the ast_type_traits namespace. (authored by alexfh).
Remove the ast_type_traits namespace.
Dec 14 2020, 5:16 PM
alexfh closed D93244: Remove the ast_type_traits namespace..
Dec 14 2020, 5:16 PM · Restricted Project
alexfh requested review of D93244: Remove the ast_type_traits namespace..
Dec 14 2020, 2:09 PM · Restricted Project

Dec 11 2020

alexfh committed rG4c5e0c7fd801: Remove references to the ast_type_traits namespace (authored by alexfh).
Remove references to the ast_type_traits namespace
Dec 11 2020, 3:41 AM

Dec 10 2020

alexfh committed rG027899dab6ac: Remove references to the ast_type_traits namespace (authored by alexfh).
Remove references to the ast_type_traits namespace
Dec 10 2020, 3:59 PM
alexfh closed D92994: Remove references to the ast_type_traits namespace.
Dec 10 2020, 3:59 PM · Restricted Project
alexfh updated the diff for D92994: Remove references to the ast_type_traits namespace.

Removed unrelated whitespace changes.

Dec 10 2020, 3:56 PM · Restricted Project
alexfh added a reviewer for D92994: Remove references to the ast_type_traits namespace: sammccall.
Dec 10 2020, 4:36 AM · Restricted Project

Dec 9 2020

alexfh requested review of D92994: Remove references to the ast_type_traits namespace.
Dec 9 2020, 7:45 PM · Restricted Project

Nov 12 2020

alexfh committed rG76b6cb515b2f: Fix unused variable warning in release builds (authored by alexfh).
Fix unused variable warning in release builds
Nov 12 2020, 9:14 AM
alexfh committed rGa196e8092a9e: [lld] Use temporary directory to create test outputs (authored by alexfh).
[lld] Use temporary directory to create test outputs
Nov 12 2020, 5:24 AM

Oct 26 2020

alexfh added a comment to D80499: Remove obsolete ignore*() matcher uses.

@alexfh This change is based on the behavior of AST Matchers being changed to ignore invisible/implicit AST nodes by default. As the default was not changed in the end, this patch would need to be updated to add traverse(TK_IgnoreUnlessSpelledInSource) wrapping around the matchers.

Before I update the patch to add that, do you have any feedback?

Oct 26 2020, 4:04 AM · Restricted Project

Oct 22 2020

alexfh accepted D89886: [clang-tidy] Fix redefinition of module in the same module.modulemap file.

Ah, btw, any chance of adding a test for this?

Oh, I was not able to create small reproducer that without including large Apple Frameworks with modules :( My hypothesis that it is side effect of module cache that triggers module load before it is referenced from sources. I tested it on reproducer from PR47839 + my real internal example.

Oct 22 2020, 3:06 PM · Restricted Project, Restricted Project
alexfh committed rG37558fd29ee0: [clang-tidy] Add links to check docs in comments (authored by alexfh).
[clang-tidy] Add links to check docs in comments
Oct 22 2020, 4:31 AM
alexfh added inline comments to D89380: [clang-tidy] Fix for cppcoreguidelines-prefer-member-initializer to handle classes declared in macros.
Oct 22 2020, 4:23 AM · Restricted Project, Restricted Project
alexfh added a comment to D89886: [clang-tidy] Fix redefinition of module in the same module.modulemap file.

Ah, btw, any chance of adding a test for this?

Oct 22 2020, 4:12 AM · Restricted Project, Restricted Project
alexfh accepted D89886: [clang-tidy] Fix redefinition of module in the same module.modulemap file.

Looks good!

Oct 22 2020, 4:11 AM · Restricted Project, Restricted Project

Oct 15 2020

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

LG

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

Oct 14 2020

alexfh requested changes to D89380: [clang-tidy] Fix for cppcoreguidelines-prefer-member-initializer to handle classes declared in macros.
Oct 14 2020, 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.

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

Oct 13 2020

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

Oct 12 2020

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

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

Looks good!

Oct 12 2020, 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.

Oct 12 2020, 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.

Oct 12 2020, 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.
Oct 12 2020, 6:06 AM

Oct 9 2020

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

Sep 28 2020

alexfh committed rGfdfe324da195: [clang-tidy] IncludeInserter: allow <> in header name (authored by alexfh).
[clang-tidy] IncludeInserter: allow <> in header name
Sep 28 2020, 6:14 AM
alexfh closed D85666: [clang-tidy] IncludeInserter: allow <> in header name.
Sep 28 2020, 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
Sep 28 2020, 5:59 AM · Restricted Project
alexfh added inline comments to D85666: [clang-tidy] IncludeInserter: allow <> in header name.
Sep 28 2020, 5:18 AM · Restricted Project
alexfh updated the diff for D85666: [clang-tidy] IncludeInserter: allow <> in header name.
  • clang-format
  • addressed review comments
Sep 28 2020, 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