alexfh (Alexander Kornienko)
User

Projects

User does not belong to any projects.

User Details

User Since
Jul 10 2012, 10:35 AM (275 w, 5 d)

Recent Activity

Fri, Oct 20

alexfh added a comment to D39121: [clang-tidy] Misplaced Operator in Strlen in Alloc.

Apart from all other comments, I think, this check would better be placed under bugprone/.

Fri, Oct 20, 4:59 PM · Restricted Project
alexfh added inline comments to D39105: Patch bug 27628 (clang-tidy should exit with a failure code when there are errors).
Fri, Oct 20, 4:07 PM
alexfh requested changes to D39105: Patch bug 27628 (clang-tidy should exit with a failure code when there are errors).

This patch breaks a number of tests. Please run the tests and fix the failures.

Fri, Oct 20, 4:03 PM
alexfh added inline comments to D38549: [clang-tidy] Link targets and Asm parsers.
Fri, Oct 20, 3:30 PM
alexfh added inline comments to D38549: [clang-tidy] Link targets and Asm parsers.
Fri, Oct 20, 3:24 PM
alexfh added inline comments to D39141: [clang-tidy] New argument --language to add_new_check.py.
Fri, Oct 20, 3:10 PM
alexfh added inline comments to D39141: [clang-tidy] New argument --language to add_new_check.py.
Fri, Oct 20, 2:49 PM
alexfh accepted D39105: Patch bug 27628 (clang-tidy should exit with a failure code when there are errors).

Looks good! Thank you!

Fri, Oct 20, 2:39 PM
alexfh added a comment to D39142: [clang-tidy ObjC] [3/3] New check objc-forbidden-subclassing.
  1. Should we make a separate module for ObjectiveC checks?
  2. Is this check actually Google-style-specific?
Fri, Oct 20, 2:29 PM
alexfh added a comment to D39141: [clang-tidy] New argument --language to add_new_check.py.

A higher-level option - language (maybe with an optional standard, e.g. c++11 or c11) - would probably be more useful, since we could also use it to modify the template of the check to bail out on other languages.

Fri, Oct 20, 2:27 PM

Wed, Oct 18

alexfh added a comment to D38549: [clang-tidy] Link targets and Asm parsers.

Zachary, do you plan to commit this? I'd also suggest adding a regression test.

Wed, Oct 18, 12:23 PM

Fri, Oct 13

alexfh added inline comments to D38284: [clang-tidy] Fix google-readability-namespace-comments handling of C++17 nested namespaces.
Fri, Oct 13, 7:24 AM · Restricted Project
alexfh committed rL315682: [clang-tidy] Add a regression test for google-readability-namespace-comments.
[clang-tidy] Add a regression test for google-readability-namespace-comments
Fri, Oct 13, 7:11 AM
alexfh added inline comments to D38284: [clang-tidy] Fix google-readability-namespace-comments handling of C++17 nested namespaces.
Fri, Oct 13, 6:14 AM · Restricted Project

Thu, Oct 12

alexfh added inline comments to D38284: [clang-tidy] Fix google-readability-namespace-comments handling of C++17 nested namespaces.
Thu, Oct 12, 7:25 AM · Restricted Project
alexfh committed rL315580: Revert "Fix nested namespaces in google-readability-nested-namespace-comments.".
Revert "Fix nested namespaces in google-readability-nested-namespace-comments."
Thu, Oct 12, 7:25 AM
alexfh added inline comments to D38284: [clang-tidy] Fix google-readability-namespace-comments handling of C++17 nested namespaces.
Thu, Oct 12, 7:06 AM · Restricted Project
alexfh committed rL315574: Fix the google-readability-namespace-comments-cxx17 test after r315060..
Fix the google-readability-namespace-comments-cxx17 test after r315060.
Thu, Oct 12, 3:41 AM

Thu, Oct 5

alexfh accepted D38549: [clang-tidy] Link targets and Asm parsers.

The first attempt to fix this (https://reviews.llvm.org/D17981) was stuck on figuring out whether we care about the binary size. It seems to me that the binary size is secondary to maintaining proper functionality for platforms that use inline assembly in headers. Thus, LG

Thu, Oct 5, 5:57 PM

Wed, Oct 4

alexfh committed rL314895: Fix assertion failure in thread safety analysis (PR34800)..
Fix assertion failure in thread safety analysis (PR34800).
Wed, Oct 4, 3:26 AM
alexfh closed D38458: Fix assertion failure in thread safety analysis (PR34800). by committing rL314895: Fix assertion failure in thread safety analysis (PR34800)..
Wed, Oct 4, 3:26 AM
alexfh updated the diff for D38458: Fix assertion failure in thread safety analysis (PR34800)..

Get rid of a local string variable.

Wed, Oct 4, 3:21 AM

Mon, Oct 2

alexfh added a comment to D38455: [clang-tidy] new cppcoreguidelines-narrowing-conversions check..

Before going deeper, I'd like to understand what's the difference between this check and -Wconversion group of clang warnings?

Mon, Oct 2, 7:28 AM · Restricted Project
alexfh added inline comments to D38458: Fix assertion failure in thread safety analysis (PR34800)..
Mon, Oct 2, 7:26 AM
alexfh updated the summary of D38458: Fix assertion failure in thread safety analysis (PR34800)..
Mon, Oct 2, 7:22 AM
alexfh created D38458: Fix assertion failure in thread safety analysis (PR34800)..
Mon, Oct 2, 7:11 AM

Thu, Sep 28

alexfh added a comment to D38358: [analyzer] Fix autodetection of getSVal()'s type argument..

Thank you for the fix! It seems to be the largest source of SA crashes on our code at the moment.

Thu, Sep 28, 8:41 AM

Wed, Sep 27

alexfh added inline comments to D38179: [clang-tidy] Handle unions in modernize-use-equals-default check.
Wed, Sep 27, 6:45 AM
alexfh added a comment to D37023: [analyzer] Fix bugreporter::getDerefExpr() again..

Thank you, Artem!

Wed, Sep 27, 6:38 AM

Mon, Sep 25

alexfh added a comment to D38171: Implement clang-tidy check aliases..

On a somewhat related note, since this is already talking about aliases

I feel like the current handling of the clang-tidy check aliases needs adjusting.
If one enables all the checks (Checks: '*'), and then disables some of them
on check-by-check basis,

Mon, Sep 25, 6:08 AM
alexfh requested changes to D33829: [clang-tidy] avoid reserved names check.
Mon, Sep 25, 5:30 AM · Restricted Project
alexfh added a comment to D38214: [analyzer] Fix crash on modeling of pointer arithmetic.

Thanks for the fix!

Mon, Sep 25, 5:06 AM

Sep 22 2017

alexfh added a comment to D38171: Implement clang-tidy check aliases..

András, that's definitely an interesting idea. However, it might be interesting to explore a more principled approach:

  1. Make clang-diagnostic-* checks first-class citizens and take full control of all diagnostics, i.e. disable all Clang diagnostics by default, and enable the ones that correspond to the enabled clang-diagnostic checks.
  2. Make aliases first-class citizens (there was a proposal as well, but we didn't arrive to a consensus at that time). That would include the ability to configure an alias name for any check including clang-diagnostic- and clang-analyzer- checks.
  3. Use aliases to map clang-diagnostic- checks to check names under cert-, hicpp-, etc.
Sep 22 2017, 7:18 AM

Sep 21 2017

alexfh added a comment to D36836: [clang-tidy] Implement readability-function-cognitive-complexity check.

The metric is implemented as per COGNITIVE COMPLEXITY by SonarSource specification version 1.2 (19 April 2017),

Sep 21 2017, 8:22 AM · Restricted Project
alexfh added a comment to D20689: [clang-tidy] Suspicious Call Argument checker.

@alexfh, would you mind taking a look at the changes that have been introduced in the new patch?

The main improvements are:

    • The checker has been shifted to the module readability.
    • It is checked, whether implicit type conversion is possible from the argument to the parameter.

      I have run the modified checker on some large code bases with the following results: (The errors were categorized subjectively.)
  • PostgreSQL: 32 renaming opportunities of 39 warnings
  • Cpython: 10 renaming opportunities of 15 warnings
  • Xerces: 6 renaming opportunities of 8 warnings
  • FFmpeg: 5 renaming opportunities of 9 warnings
  • OpenSSL: 3 renaming opportunities of 4 warnings
  • LLVM: 20 renaming opportunities of 44 warnings
Sep 21 2017, 8:08 AM · Restricted Project
alexfh removed a reviewer for D24192: [clang-refactor] introducing clang-refactor: alexfh.
Sep 21 2017, 8:05 AM
alexfh accepted D37263: [clang-format] Ignore case and stable sort using-declarations.

LG, thanks!

Sep 21 2017, 7:50 AM

Sep 20 2017

alexfh committed rL313758: Revert r313736: "[SLP] Vectorize jumbled memory loads.".
Revert r313736: "[SLP] Vectorize jumbled memory loads."
Sep 20 2017, 7:54 AM
alexfh committed rL313757: Revert r313753: "Fix a -Wsign-compare warning in LoopAccessAnalysis.cpp".
Revert r313753: "Fix a -Wsign-compare warning in LoopAccessAnalysis.cpp"
Sep 20 2017, 7:54 AM
alexfh committed rL313753: Fix a -Wsign-compare warning in LoopAccessAnalysis.cpp.
Fix a -Wsign-compare warning in LoopAccessAnalysis.cpp
Sep 20 2017, 5:20 AM
alexfh committed rL313752: [clang-tidy] Fix linkage-related compiler errors in clang-tidy tests.
[clang-tidy] Fix linkage-related compiler errors in clang-tidy tests
Sep 20 2017, 5:18 AM
alexfh added a comment to D37978: [analyzer] Fix an assertion fail in VirtualCallChecker.

Actually, after looking at the getBestDynamicClassType() implementation, I'm pretty sure it does all things IgnoreImpCasts() used to do that are relevant to the use case. So the patch seems to be trivially correct. Other reviewers' comments (if any) can be addressed post-commit. Thanks again for the fix!

Sep 20 2017, 2:10 AM

Sep 19 2017

alexfh added a comment to D37263: [clang-format] Ignore case and stable sort using-declarations.

As discussed offline, the sorting should also be stable to avoid no-op replacements for identical using declarations and randomizing the order of using declarations differing only in case (not that I'd expect this to be a frequent thing, but it's better to handle it properly anyway).

Sep 19 2017, 3:30 AM

Sep 18 2017

alexfh accepted D37978: [analyzer] Fix an assertion fail in VirtualCallChecker.

Looks reasonable, but please wait for someone who actually knows this code.

Sep 18 2017, 8:58 AM
alexfh added a comment to D37023: [analyzer] Fix bugreporter::getDerefExpr() again..

Any news here? I'm wondering mainly because this patch is supposed to fix https://bugs.llvm.org/show_bug.cgi?id=34373.

Sep 18 2017, 7:04 AM

Sep 15 2017

alexfh committed rL313357: Remove unneeded forward declaration. NFC.
Remove unneeded forward declaration. NFC
Sep 15 2017, 4:47 AM
alexfh added a comment to D37563: Convenience/safety fix for llvm::sys::Execute(And|No)Wait.
In D37563#871083, @vsk wrote:

Actually could you add a blurb about this to docs/ReleaseNotes? I had to fix some old code in lldb and stuff we have internally. It'd be useful to add a sentence or two about how to upgrade old callsites.

Sep 15 2017, 4:47 AM
alexfh committed rL313356: Add a ReleaseNotes blurb for Execute.*Wait API change.
Add a ReleaseNotes blurb for Execute.*Wait API change
Sep 15 2017, 4:46 AM
alexfh committed rL313355: [clang-tidy] Fixed misc-unused-parameters omitting parameters square brackets.
[clang-tidy] Fixed misc-unused-parameters omitting parameters square brackets
Sep 15 2017, 4:30 AM
alexfh closed D37846: [clang-tidy] Fixed misc-unused-parameters omitting parameters square brackets by committing rL313355: [clang-tidy] Fixed misc-unused-parameters omitting parameters square brackets.
Sep 15 2017, 4:30 AM · Restricted Project
alexfh accepted D37846: [clang-tidy] Fixed misc-unused-parameters omitting parameters square brackets.

LG. I'll commit the patch for you.

Sep 15 2017, 4:11 AM · Restricted Project

Sep 14 2017

alexfh added inline comments to D37846: [clang-tidy] Fixed misc-unused-parameters omitting parameters square brackets.
Sep 14 2017, 5:11 AM · Restricted Project
alexfh added a comment to D37846: [clang-tidy] Fixed misc-unused-parameters omitting parameters square brackets.

Once we got that far, could you also ensure this works for function pointer arguments (e.g. void f(void (*fn)()) {})?

Sep 14 2017, 5:03 AM · Restricted Project
alexfh added a comment to D37563: Convenience/safety fix for llvm::sys::Execute(And|No)Wait.
In D37563#865541, @vsk wrote:

It'd be nice to add in "expected 3 redirects" to the assert bodies.

Sep 14 2017, 4:56 AM

Sep 13 2017

alexfh committed rL313162: Attempt to fix MSVC build..
Attempt to fix MSVC build.
Sep 13 2017, 10:47 AM
alexfh committed rL313156: Update users of llvm::sys::ExecuteAndWait etc..
Update users of llvm::sys::ExecuteAndWait etc.
Sep 13 2017, 10:05 AM
alexfh closed D37564: Update users of llvm::sys::ExecuteAndWait etc. by committing rL313156: Update users of llvm::sys::ExecuteAndWait etc..
Sep 13 2017, 10:05 AM
alexfh committed rL313155: Convenience/safety fix for llvm::sys::Execute(And|No)Wait.
Convenience/safety fix for llvm::sys::Execute(And|No)Wait
Sep 13 2017, 10:05 AM
alexfh closed D37563: Convenience/safety fix for llvm::sys::Execute(And|No)Wait by committing rL313155: Convenience/safety fix for llvm::sys::Execute(And|No)Wait.
Sep 13 2017, 10:05 AM
alexfh added a comment to D37566: [clang-tidy] fixed misc-unused-parameters omitting parameters default value.

Done :) Could you please commit this for me?

Sep 13 2017, 7:57 AM · Restricted Project
alexfh committed rL313150: [clang-tidy] fixed misc-unused-parameters omitting parameters default value.
[clang-tidy] fixed misc-unused-parameters omitting parameters default value
Sep 13 2017, 7:56 AM
alexfh closed D37566: [clang-tidy] fixed misc-unused-parameters omitting parameters default value by committing rL313150: [clang-tidy] fixed misc-unused-parameters omitting parameters default value.
Sep 13 2017, 7:56 AM · Restricted Project
alexfh added inline comments to D37563: Convenience/safety fix for llvm::sys::Execute(And|No)Wait.
Sep 13 2017, 7:53 AM

Sep 11 2017

alexfh accepted D37572: [clang-tidy] SuspiciousEnumUsageCheck bugfix.

Still LG. Fine to commit.

Sep 11 2017, 6:50 AM

Sep 10 2017

alexfh removed a reviewer for D33825: [clang-tidy] signal handler must be plain old function check: alexfh.
Sep 10 2017, 6:00 AM · Restricted Project
alexfh accepted D37664: [clang-tidy] add more aliases for the hicpp module.

Looks reasonable. One comment inline.

Sep 10 2017, 5:54 AM

Sep 8 2017

alexfh added inline comments to D37572: [clang-tidy] SuspiciousEnumUsageCheck bugfix.
Sep 8 2017, 10:44 AM
alexfh added inline comments to D37572: [clang-tidy] SuspiciousEnumUsageCheck bugfix.
Sep 8 2017, 10:39 AM
alexfh accepted D37572: [clang-tidy] SuspiciousEnumUsageCheck bugfix.

LG modulo comments. Thank you for the fix!

Sep 8 2017, 8:37 AM
alexfh accepted D37566: [clang-tidy] fixed misc-unused-parameters omitting parameters default value.

Thank you for the fix!

Sep 8 2017, 7:31 AM · Restricted Project

Sep 7 2017

alexfh updated the diff for D37563: Convenience/safety fix for llvm::sys::Execute(And|No)Wait.

Make default arguments consistent (None -> {}).

Sep 7 2017, 5:06 AM
alexfh created D37564: Update users of llvm::sys::ExecuteAndWait etc..
Sep 7 2017, 5:05 AM
alexfh created D37563: Convenience/safety fix for llvm::sys::Execute(And|No)Wait.
Sep 7 2017, 5:03 AM

Sep 6 2017

alexfh committed rL312646: Minor style fixes in lib/Support/**/Program.(inc|cpp)..
Minor style fixes in lib/Support/**/Program.(inc|cpp).
Sep 6 2017, 9:29 AM
alexfh requested changes to D37479: run-clang-tidy: Report progress.
Sep 6 2017, 6:14 AM

Aug 28 2017

alexfh accepted D37188: [clang-tools-extra] [cmake] Support running extra clang tool tests without static analyzer.

I might be missing something but currently clang-tidy is not built at all when static analyzer is disabled.

Aug 28 2017, 5:36 PM
alexfh added a comment to D37188: [clang-tools-extra] [cmake] Support running extra clang tool tests without static analyzer.

IIUC, these are the only clang-tidy tests that need static analyzer:

Aug 28 2017, 8:12 AM
alexfh added a comment to D37188: [clang-tools-extra] [cmake] Support running extra clang tool tests without static analyzer.

This could be done even more granularly, since clang-tidy only needs static analyzer to run clang-analyzer-* checks. But that would involve some careful #ifdef-ing of the parts of clang-tidy/ClangTidy.cpp, so I'll understand if you say you have little interest in this.

Aug 28 2017, 8:05 AM

Aug 25 2017

alexfh accepted D37138: Make run-clang-tidy compatible with Python 3.x.

Please don't forget go add cfe-commits to subscribers when you create a differential revision.

Aug 25 2017, 6:51 AM · Restricted Project
alexfh updated subscribers of D37138: Make run-clang-tidy compatible with Python 3.x.
Aug 25 2017, 6:45 AM · Restricted Project
alexfh added a comment to D37120: [analyzer] Fix modeling arithmetic.

Thank you for the prompt fix!

Aug 25 2017, 1:58 AM

Aug 24 2017

alexfh added a comment to D36564: [analyzer] Fix SimpleSValBuilder::simplifySVal.

I suspect this might have caused http://llvm.org/PR34309. Could you take a look?

Aug 24 2017, 6:40 AM
alexfh added a comment to D36564: [analyzer] Fix SimpleSValBuilder::simplifySVal.

I suspect this might have caused http://llvm.org/PR34309. Could you take a look?

Aug 24 2017, 6:28 AM
alexfh accepted D37066: [clang-tidy] A follow-up fix of braced-init-list constructors in make-unique check..

LG. Thanks!

Aug 24 2017, 5:44 AM
alexfh committed rL311651: [clang-tidy] bugprone-undefined-memory-manipulation: include type into the….
[clang-tidy] bugprone-undefined-memory-manipulation: include type into the…
Aug 24 2017, 5:12 AM

Aug 23 2017

alexfh added inline comments to D37066: [clang-tidy] A follow-up fix of braced-init-list constructors in make-unique check..
Aug 23 2017, 9:20 AM

Aug 17 2017

alexfh committed rL311136: [clang-tidy] Add modernize-use-equals-default.IgnoreMacros option.
[clang-tidy] Add modernize-use-equals-default.IgnoreMacros option
Aug 17 2017, 4:09 PM
alexfh accepted D36670: misc-misplaced-widening-cast: fix assertion.

LG. Thank you for the fix!

Aug 17 2017, 7:55 AM · Restricted Project
alexfh accepted D36822: [clang-tidy] Ignore statements inside a template instantiation..

LG with a nit.

Aug 17 2017, 7:01 AM
alexfh accepted D36786: [clang-tidy] Don't generate fixes for initializer_list constructor in make_unique check..

Still LG with one comment.

Aug 17 2017, 2:50 AM

Aug 16 2017

alexfh accepted D36786: [clang-tidy] Don't generate fixes for initializer_list constructor in make_unique check..

LG

Aug 16 2017, 8:33 AM
alexfh accepted D35367: [clang-tidy] Add a close-on-exec check on epoll_create() in Android module..

LG with a nit.

Aug 16 2017, 8:30 AM · Restricted Project
alexfh accepted D35363: [clang-tidy] Add a close-on-exec check on accept4() in Android module..

LG

Aug 16 2017, 8:29 AM · Restricted Project
alexfh accepted D35365: [clang-tidy] Add a close-on-exec check on epoll_create1() in Android module..

LG

Aug 16 2017, 8:29 AM · Restricted Project
alexfh accepted D35362: [clang-tidy] Add a close-on-exec check on accept() in Android module..

LG with a nit.

Aug 16 2017, 8:29 AM · Restricted Project
alexfh added a comment to D34440: [Clang] Expand response files before loading compilation database.

@alexfh
Thanks for the response!
Yes, we re-implemented logic and now use JSON compilation database to pass compiler options to Clang-Tidy.
Anyway, i think in general it's useful to support @response_files, see my comment: https://reviews.llvm.org/D34440#813411

Aug 16 2017, 8:19 AM · Restricted Project
alexfh added a comment to D34440: [Clang] Expand response files before loading compilation database.

Many build systems normally generate response files on-fly in some circumstances (e.g. if command line is longer than some platform-imposed limit). So IMO response files should be a perfect citizen here.

friendly ping

Aug 16 2017, 4:04 AM · Restricted Project

Aug 11 2017

alexfh added a comment to D35941: Fix -Wshadow false positives with function-local classes..

IIUC, most cases where -Wshadow warnings are issued is when a declaration from an enclosing scope would be accessible if there was no declaration that shadows it. In this case the the local variable of a function would not be accessible inside the local class anyway

That's not strictly true; the variable can be accessed in unevaluated operands, and in code doing so, a -Wshadow warning might (theoretically) be useful:

void f(SomeComplexType val) {
  struct A {
    decltype(val) &ref;
    void g(int val) {
      decltype(val) *p = &ref;
    }
  } a = {val};
}
Aug 11 2017, 6:05 AM

Aug 10 2017

alexfh added a comment to D36577: [clang-tidy] add an alias hicpp-braces-around-statements.

Could you rebase the patch? It doesn't apply cleanly.

Aug 10 2017, 8:42 AM · Restricted Project
alexfh accepted D36577: [clang-tidy] add an alias hicpp-braces-around-statements.
Aug 10 2017, 8:33 AM · Restricted Project