alexfh (Alexander Kornienko)
User

Projects

User does not belong to any projects.

User Details

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

Recent Activity

Fri, Jun 23

alexfh added a comment to D33589: clang-format: consider not splitting tokens in optimization.

Daniel is a better reviewer here than myself. A few cents from me though: why do we want to make an exception for comments and not for regular code?

Fri, Jun 23, 7:37 AM
alexfh edited reviewers for D33589: clang-format: consider not splitting tokens in optimization, added: djasper; removed: alexfh.
Fri, Jun 23, 7:35 AM
alexfh accepted D34513: [NFC] Update to account for DiagnosticRenderer use of FullSourceLoc.

LG. Looks like a pretty mechanical change.

Fri, Jun 23, 6:32 AM
alexfh added a comment to D30748: [Lexer] Finding beginning of token with escaped new line.

Sorry for the delay, I was on vacation.

Fri, Jun 23, 5:09 AM
alexfh accepted D33304: [clang-tidy][Part1] Add a new module Android and three new checks..

LG with one nit.

Fri, Jun 23, 4:21 AM · Restricted Project
alexfh accepted D34526: [clang-tidy] Fix modernize-use-nullptr only warns the first NULL argument..

LG

Fri, Jun 23, 2:56 AM
alexfh accepted D34524: [clang-tidy] Fix a false positive in modernize-use-nullptr..

LG. Thank you for the fix!

Fri, Jun 23, 1:52 AM

Fri, Jun 9

alexfh added a comment to D34002: [clang-tidy] When" -fno-exceptions is used", this warning is better to be suppressed..
In D34002#776948, @chh wrote:

Thanks for providing the ODR reason for Android -fno-exceptions users to change source code or suppress this warning themselves.

Fri, Jun 9, 10:06 AM · Restricted Project
alexfh requested changes to D33304: [clang-tidy][Part1] Add a new module Android and three new checks..
Fri, Jun 9, 8:39 AM · Restricted Project
alexfh added a comment to D34002: [clang-tidy] When" -fno-exceptions is used", this warning is better to be suppressed..
In D34002#776551, @chh wrote:

Android source is suppressing misc-noexcept-move-constructor warnings
because -fno-exceptions is used and Android does not like to add more
exception specific code.

Fri, Jun 9, 12:52 AM · Restricted Project
alexfh committed rL305057: Revert "[clang-tidy] When" -fno-exceptions is used", this warning is better to….
Revert "[clang-tidy] When" -fno-exceptions is used", this warning is better to…
Fri, Jun 9, 12:35 AM

Thu, Jun 8

alexfh added inline comments to D34002: [clang-tidy] When" -fno-exceptions is used", this warning is better to be suppressed..
Thu, Jun 8, 3:27 PM · Restricted Project
alexfh committed rL305024: [clang-tidy] Use -fexceptions explicitly in the tests that need exceptions..
[clang-tidy] Use -fexceptions explicitly in the tests that need exceptions.
Thu, Jun 8, 3:26 PM
alexfh added a comment to D34002: [clang-tidy] When" -fno-exceptions is used", this warning is better to be suppressed..
In D34002#776193, @chh wrote:

IIUC, when vector<T> (for a class T that has both move and copy constructors) resizes, it will prefer move constructors, but only if they're declared noexcept. This is true even if -fno-exceptions is on. So I don't think this check should depend on -fno-exceptions.

Should the compiler assume noexcept when -fno-exceptions is on?
That means move constructors should be preferred under -fno-exceptions, and this check would be unnecessary, right?

Thu, Jun 8, 3:24 PM · Restricted Project
alexfh committed rL304977: [clang-tidy] New checker to replace dynamic exception specifications.
[clang-tidy] New checker to replace dynamic exception specifications
Thu, Jun 8, 7:05 AM
alexfh closed D20693: [clang-tidy] New checker to replace dynamic exception specifications by committing rL304977: [clang-tidy] New checker to replace dynamic exception specifications.
Thu, Jun 8, 7:04 AM
alexfh added a comment to D20693: [clang-tidy] New checker to replace dynamic exception specifications.

Great, thanks for you help.

Could you commit this for me?

Thu, Jun 8, 6:43 AM
alexfh added a comment to D33365: [clang-tidy] misc-assertion-count: A New Check.

I guess, this check should go to readability or elsewhere, but definitely not to misc.

Hmm, misc may be a bad place for this, but i think readability is even worse fit.
The best guess would be something like hardening / security, but there is no such category.

Thu, Jun 8, 3:52 AM · Restricted Project
alexfh requested changes to D33365: [clang-tidy] misc-assertion-count: A New Check.

I guess, this check should go to readability or elsewhere, but definitely not to misc.

Thu, Jun 8, 1:23 AM · Restricted Project
alexfh added a comment to D33722: [clang-tidy] Add checker for undelegated copy of base classes.

I would be interested in seeing the results of this check's run on LLVM+Clang code.

Thu, Jun 8, 1:00 AM · Restricted Project
alexfh added a comment to D34002: [clang-tidy] When" -fno-exceptions is used", this warning is better to be suppressed..

Here's a small test: https://godbolt.org/g/nNZgUb (look for T::T( and S::S( calls, adding -fno-exceptions doesn't change which constructors are called).

Thu, Jun 8, 12:36 AM · Restricted Project
alexfh added a comment to D34002: [clang-tidy] When" -fno-exceptions is used", this warning is better to be suppressed..

IIUC, when vector<T> (for a class T that has both move and copy constructors) resizes, it will prefer move constructors, but only if they're declared noexcept. This is true even if -fno-exceptions is on. So I don't think this check should depend on -fno-exceptions.

Thu, Jun 8, 12:24 AM · Restricted Project
alexfh accepted D20693: [clang-tidy] New checker to replace dynamic exception specifications.

Now that you found the root cause, this looks like a proper fix ;)

Thu, Jun 8, 12:08 AM

Wed, Jun 7

alexfh requested changes to D33841: [clang-tidy] redundant keyword check.
Wed, Jun 7, 12:43 PM · Restricted Project
alexfh added a comment to D20693: [clang-tidy] New checker to replace dynamic exception specifications.

I have not, as yet, been able to reproduce the buildbot failures. They were essentially intermittent seg-faults, and corrupt diag output.

I will work on creating a test that can reproduce the problem.

Wed, Jun 7, 12:28 PM
alexfh added a comment to D20693: [clang-tidy] New checker to replace dynamic exception specifications.
  • Only pass %2 parameter if %2 is included in format.

I thought, DiagnosticsBuilder handles placeholders in conditional parts correctly. Did you find an evidence of the opposite? Can you add a test that consistently fails?

Wed, Jun 7, 8:20 AM
alexfh added a comment to D20693: [clang-tidy] New checker to replace dynamic exception specifications.
  • Only pass %2 parameter if %2 is included in format.
Wed, Jun 7, 8:19 AM
alexfh accepted D33982: [clang-format] Fix alignment of preprocessor trailing comments.

LG

Wed, Jun 7, 5:55 AM
alexfh updated subscribers of D33982: [clang-format] Fix alignment of preprocessor trailing comments.
Wed, Jun 7, 5:54 AM
alexfh requested changes to D20693: [clang-tidy] New checker to replace dynamic exception specifications.

In any case, I'm strongly against these hacks, please revert them.

Wed, Jun 7, 1:48 AM
alexfh added a comment to D20693: [clang-tidy] New checker to replace dynamic exception specifications.

In order to fix diagnostic corruption in some of the buildbot tests
(unable to reproduce locally):

  • make NoexceptMacro a static variable so it's lifetime doesn't end when UseNoexceptCheck is destroyed.
  • pass a const char* instead of a StringRef to DiagnosticBuilder so it won't create a temporary std::string and cache the address of the temporary char * it owns.
Wed, Jun 7, 1:48 AM
alexfh committed rL304879: [clang-tidy] Make misc-inaccurate-erase work with real C++11 containers..
[clang-tidy] Make misc-inaccurate-erase work with real C++11 containers.
Wed, Jun 7, 1:26 AM

Tue, Jun 6

alexfh committed rL304811: [clang-tidy] misc-inaccurate-erase: support call by pointer.
[clang-tidy] misc-inaccurate-erase: support call by pointer
Tue, Jun 6, 10:50 AM

Fri, Jun 2

alexfh accepted D33827: [clang-tidy] misc-static-assert shouldn't flag assert(!"msg").

LG. Thank you!

Fri, Jun 2, 12:37 PM · Restricted Project
alexfh committed rL304583: [clang-tidy] Add `const` to operator() to fix a warning..
[clang-tidy] Add `const` to operator() to fix a warning.
Fri, Jun 2, 11:48 AM
alexfh committed rL304581: Fix formatting in docs..
Fix formatting in docs.
Fri, Jun 2, 11:44 AM
alexfh removed a reviewer for D19201: [clang-tidy] misc-throw-with-noexcept: alexfh.
Fri, Jun 2, 10:38 AM
alexfh committed rL304570: [clang-tidy] check for __func__/__FUNCTION__ in lambdas.
[clang-tidy] check for __func__/__FUNCTION__ in lambdas
Fri, Jun 2, 10:37 AM
alexfh closed D33497: [clang-tidy] check for __func__/__FUNCTION__ in lambdas by committing rL304570: [clang-tidy] check for __func__/__FUNCTION__ in lambdas.
Fri, Jun 2, 10:36 AM · Restricted Project
alexfh accepted D33497: [clang-tidy] check for __func__/__FUNCTION__ in lambdas.

Looks good!

Fri, Jun 2, 10:05 AM · Restricted Project
alexfh added a comment to D33304: [clang-tidy][Part1] Add a new module Android and three new checks..

Thanks for the contributions.

All your three checks are not android specific -- because they are checking POSIX APIs (open, creat, fopen), which are more likely related to POSIX. So personally, I'm +1 on a "posix" module, instead of "android", but wait to see other reviewers' opinions before renaming it.

Fri, Jun 2, 9:27 AM · Restricted Project
alexfh accepted D33830: [clang-format] Don't align too long broken trailing comments.
Fri, Jun 2, 9:05 AM
alexfh added a comment to D33830: [clang-format] Don't align too long broken trailing comments.

LG

Fri, Jun 2, 9:05 AM

Thu, Jun 1

alexfh committed rL304408: Make the clang-cl test less restrictive..
Make the clang-cl test less restrictive.
Thu, Jun 1, 4:41 AM
alexfh added a comment to D33560: [clang-format] Add option to specify explicit config file.

Please add a test for the new -style syntax.

Thu, Jun 1, 3:25 AM

Wed, May 31

alexfh added a comment to D33304: [clang-tidy][Part1] Add a new module Android and three new checks..

It's awesome to see another two checks, and it seems to justify adding a separate module, but I'd prefer the other two checks to be sent for review as separate patches to make the review easier and faster.

The other two checks are relatively small and simple. I understand this may make the review process longer and we could wait for that. Please take your time :-) Thank you!

Wed, May 31, 10:59 AM · Restricted Project
alexfh committed rL304302: Enable the ARM Neon intrinsics test by default..
Enable the ARM Neon intrinsics test by default.
Wed, May 31, 7:36 AM
alexfh added a reverting commit for rL304208: [ARM] Update long-test after r304201.: rL304301: Revert "[ARM] Update long-test after r304201.".
Wed, May 31, 7:33 AM
alexfh committed rL304301: Revert "[ARM] Update long-test after r304201.".
Revert "[ARM] Update long-test after r304201."
Wed, May 31, 7:33 AM
alexfh added a comment to D33304: [clang-tidy][Part1] Add a new module Android and three new checks..

It's awesome to see another two checks, and it seems to justify adding a separate module, but I'd prefer the other two checks to be sent for review as separate patches to make the review easier and faster.

Wed, May 31, 5:59 AM · Restricted Project

Tue, May 30

alexfh added a comment to D33531: Clang-tidy readability: avoid const value return.

Would it make sense to silence this diagnostic in the presence of also checking for cert-dcl21-cpp for such operators?

Currently there's no mechanism in clang-tidy to express dependencies or compatibility issues between checks. Moreover, we already have checks that are not meant to be run together, for example, readability-braces-around-statements and its google- incarnation (and other alias checks with different settings). That said, we could whitelist postfix increment and decrement operators in this check. Camillo, WDYT?

I can imagine a generic whitelist mechanism might be useful for this check. It could even be empty by default, but the documentation could call out cert-dcl21-cpp specifically and show an example of how you can run both checks.

Tue, May 30, 6:14 AM · Restricted Project
alexfh added inline comments to D33497: [clang-tidy] check for __func__/__FUNCTION__ in lambdas.
Tue, May 30, 5:05 AM · Restricted Project
alexfh added a comment to D33623: Make the parser close parens for you on EOF.

Sam knows this stuff much better.

Tue, May 30, 2:27 AM
alexfh edited reviewers for D33623: Make the parser close parens for you on EOF, added: sbenza; removed: alexfh.
Tue, May 30, 2:26 AM

Mon, May 29

alexfh committed rL304154: [clang-tidy] Use getLocalOrGlobal for the StrictMode option.
[clang-tidy] Use getLocalOrGlobal for the StrictMode option
Mon, May 29, 6:59 AM
alexfh added a comment to D33531: Clang-tidy readability: avoid const value return.

Would it make sense to silence this diagnostic in the presence of also checking for cert-dcl21-cpp for such operators?

Mon, May 29, 6:49 AM · Restricted Project
alexfh added inline comments to D33531: Clang-tidy readability: avoid const value return.
Mon, May 29, 4:57 AM · Restricted Project
alexfh added a comment to D33531: Clang-tidy readability: avoid const value return.

The check ignores returns of const pointers (meaning * const, not const *); while pointless, these are not particularly harmful. It could be made laxer by ignoring all trivially copyable types (on the grounds that they won't have interesting move constructors/assigners anyway), or stricter by ignoring all const returns (on the grounds that, in the best case, it's just pointless verbosity). Or it could be made an option.

Mon, May 29, 4:42 AM · Restricted Project
alexfh updated subscribers of D33531: Clang-tidy readability: avoid const value return.
Mon, May 29, 4:33 AM · Restricted Project

May 23 2017

alexfh accepted D33272: Method loadFromCommandLine should be able to report errors.

Thanks, LG!

May 23 2017, 8:42 AM
alexfh added inline comments to D33272: Method loadFromCommandLine should be able to report errors.
May 23 2017, 3:23 AM

May 22 2017

alexfh committed rL303554: [clang-tidy] misc-move-const-arg shouldn't complain on std::move(lambda).
[clang-tidy] misc-move-const-arg shouldn't complain on std::move(lambda)
May 22 2017, 7:30 AM
alexfh committed rL303552: [clang-tidy] readability-redundant-declaration false positive for defaulted….
[clang-tidy] readability-redundant-declaration false positive for defaulted…
May 22 2017, 6:59 AM
alexfh closed D33358: [clang-tidy] readability-redundant-declaration false positive for defaulted function by committing rL303552: [clang-tidy] readability-redundant-declaration false positive for defaulted….
May 22 2017, 6:59 AM
alexfh committed rL303551: [clang-tidy] readability-braces-around-statements false positive with char….
[clang-tidy] readability-braces-around-statements false positive with char…
May 22 2017, 6:58 AM
alexfh closed D33354: [clang-tidy] readability-braces-around-statements false positive with char literals by committing rL303551: [clang-tidy] readability-braces-around-statements false positive with char….
May 22 2017, 6:58 AM
alexfh requested changes to D32346: [clang-tidy] New readability check for strlen argument.
May 22 2017, 6:48 AM
alexfh added inline comments to D32346: [clang-tidy] New readability check for strlen argument.
May 22 2017, 6:41 AM
alexfh removed a reviewer for D31130: B32239 clang-tidy should not warn about array to pointer decay on system macros: alexfh.
May 22 2017, 6:33 AM
alexfh added inline comments to D33272: Method loadFromCommandLine should be able to report errors.
May 22 2017, 6:31 AM
alexfh accepted D33272: Method loadFromCommandLine should be able to report errors.

Thank you! This looks good with one nit (see the inline comment).

May 22 2017, 6:27 AM
alexfh requested changes to D22500: FileCheck Enhancement - Including files.

IIUC, there was no consensus about usefulness of this change.

May 22 2017, 6:07 AM
alexfh requested changes to D22502: FileCheck Enhancement - CHECK-LABEL-DAG.

IIUC, there was no consensus about usefulness of this change.

May 22 2017, 6:06 AM
alexfh requested changes to D22503: FileCheck Enhancement - prefixes-regular expressions.

IIUC, there was no consensus about usefulness of this change.

May 22 2017, 6:05 AM
alexfh requested changes to D33304: [clang-tidy][Part1] Add a new module Android and three new checks..
May 22 2017, 6:00 AM · Restricted Project
alexfh added inline comments to D33102: [clang] Implement -Wcast-qual for C++.
May 22 2017, 5:43 AM
alexfh added a comment to D33354: [clang-tidy] readability-braces-around-statements false positive with char literals.

I'm going to commit the patch for you, but I guess, you might want to ask for commit access (http://llvm.org/docs/DeveloperPolicy.html#obtaining-commit-access).

May 22 2017, 5:39 AM
alexfh accepted D33354: [clang-tidy] readability-braces-around-statements false positive with char literals.

LG. Thank you for investigating this!

May 22 2017, 5:33 AM

May 19 2017

alexfh added a comment to D33354: [clang-tidy] readability-braces-around-statements false positive with char literals.

I'll repeat my comment from D25558: "I'm thoroughly confused as to why the code in your test was not handled correctly and why this is the right fix. Can you explain?" The "For some reason ..." part doesn't really explain anything. I guess, we're papering over a more generic problem, and someone has to figure out what it is and how to fix it properly.

May 19 2017, 8:35 AM
alexfh accepted D33358: [clang-tidy] readability-redundant-declaration false positive for defaulted function.

LG. Thank you!

May 19 2017, 8:30 AM

May 18 2017

alexfh added a comment to D33304: [clang-tidy][Part1] Add a new module Android and three new checks..

I find the use of "must" at the very least inappropriate. If there was no use case for not including it, it wouldn't be an option. There is also nothing really Android-specific here beside maybe the open64 mess.

On Android, we are requiring this flag. That is why this is part of a new category of Android-specific tidy rules. If you think this belongs more generally in a different category for tidy, can you suggest somewhere else to put it? We didn't want to impose these restrictions for platforms that might not want to be so strict. Also, as with any static analysis, there is the possibility that the original code author intended to "break" the rules, but that is what NOLINT is for.

I'm not keen on putting this in an Android module either, as it's not really Android-specific behavior. For instance, this is also part of a recommended compliant solution for CERT FIO22-C.

May 18 2017, 11:43 AM · Restricted Project

May 17 2017

alexfh committed rL303325: Fix an assertion failure in FormatASTNodeDiagnosticArgument..
Fix an assertion failure in FormatASTNodeDiagnosticArgument.
May 17 2017, 8:15 PM
alexfh closed D33207: Fix an assertion failure in FormatASTNodeDiagnosticArgument. by committing rL303325: Fix an assertion failure in FormatASTNodeDiagnosticArgument..
May 17 2017, 8:15 PM
alexfh committed rL303321: [clang-tidy] Optimize GlobList::contains.
[clang-tidy] Optimize GlobList::contains
May 17 2017, 6:27 PM
alexfh committed rL303264: Change getChecksFilter() interface to hide implementation details..
Change getChecksFilter() interface to hide implementation details.
May 17 2017, 7:55 AM
alexfh committed rL303263: [clang-tidy] Replace matchesName with hasName where no regex is needed.
[clang-tidy] Replace matchesName with hasName where no regex is needed
May 17 2017, 7:55 AM
alexfh committed rL303256: [clang-tidy] A bit of refactoring of modernize-replace-auto-ptr. NFC.
[clang-tidy] A bit of refactoring of modernize-replace-auto-ptr. NFC
May 17 2017, 6:10 AM
alexfh added a comment to D32700: [clang-tidy] Add misc-suspicious-memset-usage check..

The existing google-runtime-memset-zero-length check is related. It finds cases when the byte count parameter is zero and offers to swap that with the fill value argument. Perhaps the two could be merged while maintaining backward compatibility through an alias.

May 17 2017, 5:04 AM · Restricted Project
alexfh added inline comments to D33102: [clang] Implement -Wcast-qual for C++.
May 17 2017, 4:47 AM

May 16 2017

alexfh committed rL303230: [clang-tidy] Optimize misc-unused-parameters. NFCI.
[clang-tidy] Optimize misc-unused-parameters. NFCI
May 16 2017, 7:38 PM
alexfh accepted D32942: [clang-tidy] readability-function-size: add NestingThreshold param..

LG

May 16 2017, 4:08 PM · Restricted Project
alexfh committed rL303191: [clang-tidy] Speed up performance-unnecessary-value-param check.
[clang-tidy] Speed up performance-unnecessary-value-param check
May 16 2017, 10:41 AM
alexfh committed rL303187: [clang-tidy] Optimize readability-implicit-bool-cast, NFC.
[clang-tidy] Optimize readability-implicit-bool-cast, NFC
May 16 2017, 9:54 AM
alexfh committed rL303180: [clang-tidy] Optimize matchers in readability-implicit-bool-cast. NFC.
[clang-tidy] Optimize matchers in readability-implicit-bool-cast. NFC
May 16 2017, 8:58 AM
alexfh accepted D31700: [clang-tidy] Ignore blank spaces between cast's ")" and its sub expr..

Sorry, missed your patch somehow. LG with one nit.

May 16 2017, 7:57 AM
alexfh accepted D33209: [clang-tidy] Add "emplace_back" detection in inefficient-vector-operation..

LG once the comments are addressed.

May 16 2017, 2:48 AM · Restricted Project
alexfh added inline comments to D33209: [clang-tidy] Add "emplace_back" detection in inefficient-vector-operation..
May 16 2017, 2:46 AM · Restricted Project
alexfh added inline comments to D33207: Fix an assertion failure in FormatASTNodeDiagnosticArgument..
May 16 2017, 1:15 AM
alexfh updated the diff for D33207: Fix an assertion failure in FormatASTNodeDiagnosticArgument..

Instead of handling LinkageSpecDecl, use getRedeclContext() when issuing -Wshadow diagnostic.

May 16 2017, 1:08 AM
alexfh added inline comments to D33209: [clang-tidy] Add "emplace_back" detection in inefficient-vector-operation..
May 16 2017, 12:15 AM · Restricted Project