Page MenuHomePhabricator

vladimir.plyashkun (Vladimir Plyashkun)
User

Projects

User does not belong to any projects.

User Details

User Since
Jun 20 2017, 4:10 AM (139 w, 1 d)

Recent Activity

Oct 23 2019

vladimir.plyashkun added a comment to D68694: [clang-tidy] hicpp-signed-bitwise: Do not show "use of a signed integer operand with a binary bitwise operator" for positive integer operands.

LGTM aside from a small nit.

Oct 23 2019, 2:18 AM · Restricted Project, Restricted Project
vladimir.plyashkun updated the diff for D68694: [clang-tidy] hicpp-signed-bitwise: Do not show "use of a signed integer operand with a binary bitwise operator" for positive integer operands.
Oct 23 2019, 2:18 AM · Restricted Project, Restricted Project

Oct 15 2019

vladimir.plyashkun added a comment to D68694: [clang-tidy] hicpp-signed-bitwise: Do not show "use of a signed integer operand with a binary bitwise operator" for positive integer operands.

Do you know who is responsible for it? Because i haven't worked with documentation before and don't know what i need to do to update it.

The files reside in clang-tools-extra/docs/ReleaseNotes.rst and clang-tools-extra/docs/clang-tidy/checks/hicpp-signed-bitwise.rst.

You can check other files for reference, e.g. readability-magic-numbers.rst.
In the Release Notes you can add one entry at the end of the clang-tidy section.

Oct 15 2019, 2:37 AM · Restricted Project, Restricted Project
vladimir.plyashkun updated the diff for D68694: [clang-tidy] hicpp-signed-bitwise: Do not show "use of a signed integer operand with a binary bitwise operator" for positive integer operands.
Oct 15 2019, 2:37 AM · Restricted Project, Restricted Project

Oct 11 2019

vladimir.plyashkun added a comment to D68694: [clang-tidy] hicpp-signed-bitwise: Do not show "use of a signed integer operand with a binary bitwise operator" for positive integer operands.

The new switch needs documentation as well, and maybe even a note in the release notes (as it is publicly discussed as issue?).

Oct 11 2019, 8:14 AM · Restricted Project, Restricted Project
vladimir.plyashkun updated the diff for D68694: [clang-tidy] hicpp-signed-bitwise: Do not show "use of a signed integer operand with a binary bitwise operator" for positive integer operands.
Oct 11 2019, 8:14 AM · Restricted Project, Restricted Project
vladimir.plyashkun added inline comments to D68694: [clang-tidy] hicpp-signed-bitwise: Do not show "use of a signed integer operand with a binary bitwise operator" for positive integer operands.
Oct 11 2019, 8:14 AM · Restricted Project, Restricted Project
vladimir.plyashkun added inline comments to D68694: [clang-tidy] hicpp-signed-bitwise: Do not show "use of a signed integer operand with a binary bitwise operator" for positive integer operands.
Oct 11 2019, 4:38 AM · Restricted Project, Restricted Project
vladimir.plyashkun added a comment to D68694: [clang-tidy] hicpp-signed-bitwise: Do not show "use of a signed integer operand with a binary bitwise operator" for positive integer operands.

@aaron.ballman
@JonasToth
Thanks, i agree too! I've prepared new revision with additional option to preserve old inspection behaviour.

Oct 11 2019, 3:40 AM · Restricted Project, Restricted Project
vladimir.plyashkun updated the diff for D68694: [clang-tidy] hicpp-signed-bitwise: Do not show "use of a signed integer operand with a binary bitwise operator" for positive integer operands.

Provide additional option to preserve current inspection behaviour

Oct 11 2019, 3:33 AM · Restricted Project, Restricted Project

Oct 9 2019

vladimir.plyashkun created D68694: [clang-tidy] hicpp-signed-bitwise: Do not show "use of a signed integer operand with a binary bitwise operator" for positive integer operands.
Oct 9 2019, 4:36 AM · Restricted Project, Restricted Project

Jan 22 2018

vladimir.plyashkun updated the diff for D41535: Add -vfsoverlay option for Clang-Tidy.

Thanks, now all should be fine!

Jan 22 2018, 7:27 AM · Restricted Project
vladimir.plyashkun added a comment to D41535: Add -vfsoverlay option for Clang-Tidy.

This looks good, but we should add a test.
Should've noticed that before, sorry for the slowing this down a bit more. After the test is there, I'm happy to commit this change for you.

Jan 22 2018, 12:49 AM · Restricted Project

Jan 19 2018

vladimir.plyashkun updated the diff for D41535: Add -vfsoverlay option for Clang-Tidy.

Some more nits were fixed.

Jan 19 2018, 5:00 AM · Restricted Project
vladimir.plyashkun updated the diff for D41535: Add -vfsoverlay option for Clang-Tidy.

Fixed code review remarks.

Jan 19 2018, 2:57 AM · Restricted Project
vladimir.plyashkun updated the diff for D41947: Provide default virtual filesystem argument to ClangTool constructor.

Fixed comment

Jan 19 2018, 2:26 AM · Restricted Project
vladimir.plyashkun added a child revision for D41535: Add -vfsoverlay option for Clang-Tidy: D41947: Provide default virtual filesystem argument to ClangTool constructor.
Jan 19 2018, 2:16 AM · Restricted Project
vladimir.plyashkun added a parent revision for D41947: Provide default virtual filesystem argument to ClangTool constructor: D41535: Add -vfsoverlay option for Clang-Tidy.
Jan 19 2018, 2:16 AM · Restricted Project

Jan 18 2018

vladimir.plyashkun abandoned D41594: Support `ivfsoverlay` option in Tooling.
Jan 18 2018, 11:24 PM · Restricted Project
vladimir.plyashkun added a comment to D41947: Provide default virtual filesystem argument to ClangTool constructor.

Looks good. Do you have commit access or do you need someone to land this patch for you?

Jan 18 2018, 7:42 AM · Restricted Project
vladimir.plyashkun updated the diff for D41535: Add -vfsoverlay option for Clang-Tidy.

Moved logic to ClangTidyMain

Jan 18 2018, 3:04 AM · Restricted Project
vladimir.plyashkun updated the diff for D41947: Provide default virtual filesystem argument to ClangTool constructor.

Implemented test-case to check that BaseFS is actually used in ClangTool

Jan 18 2018, 2:57 AM · Restricted Project

Jan 17 2018

vladimir.plyashkun added a comment to D41535: Add -vfsoverlay option for Clang-Tidy.

Friendly ping

Jan 17 2018, 6:28 AM · Restricted Project

Jan 11 2018

vladimir.plyashkun retitled D41947: Provide default virtual filesystem argument to ClangTool constructor from Provide default virtual filesystem argument in ClangTool constructor to Provide default virtual filesystem argument to ClangTool constructor.
Jan 11 2018, 5:55 AM · Restricted Project
vladimir.plyashkun added a comment to D41535: Add -vfsoverlay option for Clang-Tidy.

Second part here (provided default virtual filesystem argument to ClangTool constructor): https://reviews.llvm.org/D41947

Jan 11 2018, 5:54 AM · Restricted Project
vladimir.plyashkun updated the diff for D41535: Add -vfsoverlay option for Clang-Tidy.
Jan 11 2018, 5:53 AM · Restricted Project
vladimir.plyashkun created D41947: Provide default virtual filesystem argument to ClangTool constructor.
Jan 11 2018, 5:50 AM · Restricted Project
vladimir.plyashkun abandoned D41536: Provide public getter for OverlayFileSystem.
Jan 11 2018, 5:49 AM · Restricted Project, Restricted Project

Dec 28 2017

vladimir.plyashkun added inline comments to D41594: Support `ivfsoverlay` option in Tooling.
Dec 28 2017, 2:29 AM · Restricted Project

Dec 27 2017

vladimir.plyashkun added a comment to D41535: Add -vfsoverlay option for Clang-Tidy.

FYI, i've create revision to support ivfsoverlay option in Tooling: https://reviews.llvm.org/D41594

Dec 27 2017, 5:54 AM · Restricted Project
vladimir.plyashkun created D41594: Support `ivfsoverlay` option in Tooling.
Dec 27 2017, 5:53 AM · Restricted Project

Dec 24 2017

vladimir.plyashkun added a comment to D41535: Add -vfsoverlay option for Clang-Tidy.

@ilya-biryukov
Yes, this is exactly what i need.
Unfortunately, -ivfsoverlay in the compile commands works for the compiler invocation, but it doesn't work for tooling.
E.g. this call:

clang-tidy -checks=* <file> -- -ivfsoverlay=<yaml_file>

has no effect.

Dec 24 2017, 10:35 PM · Restricted Project

Dec 21 2017

vladimir.plyashkun added a comment to D41535: Add -vfsoverlay option for Clang-Tidy.
Dec 21 2017, 11:12 PM · Restricted Project
vladimir.plyashkun created D41536: Provide public getter for OverlayFileSystem.
Dec 21 2017, 11:12 PM · Restricted Project, Restricted Project
vladimir.plyashkun created D41535: Add -vfsoverlay option for Clang-Tidy.
Dec 21 2017, 11:09 PM · Restricted Project
vladimir.plyashkun added a comment to D41290: [YAML] Add support for non-printable characters.

Hello @thegameg !
Is is correct that printable UTF-8 characters are also escaped by these changes?
For example, Clang-Tidy output now uses double-quotes for UTF-8:

Dec 21 2017, 12:30 AM

Aug 16 2017

vladimir.plyashkun 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, 5:46 AM · Restricted Project
vladimir.plyashkun abandoned D34440: [Clang] Expand response files before loading compilation database.
Aug 16 2017, 2:48 AM · Restricted Project

Jul 31 2017

vladimir.plyashkun 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

Jul 31 2017, 4:28 AM · Restricted Project

Jul 18 2017

vladimir.plyashkun added a comment to D34440: [Clang] Expand response files before loading compilation database.

Thanks for the response @klimek !
Sorry for possible confusions.

Yes. I'm still confused why in this case clang-tidy @file -- would be expected to expand the response file? Am I missing something?

I think that we discussed use-case when @response files can appear in the compiler options (right after -- symbol).

I don't understand this. Can you elaborate?

As IDE developers we can't control how users pass options to compiler.
For example, user can set compiler options directly in the CMake, like this:

Jul 18 2017, 12:51 PM · Restricted Project
vladimir.plyashkun added a comment to D34440: [Clang] Expand response files before loading compilation database.

If you want one unified format, the compilation database is it.

Is the clang-tidy ... -- <args> meant to be more or less a drop-in replacement for clang <args> (arguments-wise)?
If yes, this expansion of response files here is an another step in this direction.

Jul 18 2017, 7:14 AM · Restricted Project

Jul 14 2017

vladimir.plyashkun added a comment to D34440: [Clang] Expand response files before loading compilation database.

Are there any concerns using the alternative?

I can't say that it's a big problems, but i think that:
CompilationDatabase.json is more CMake specific format.
It can be generated automatically by CMake, while other build systems may not do it.
So we need to generate it on the fly (by tool or by hand), which also can lead to hidden problems due to different formats, different escaping rules, etc.
I think it's always good to have one unified format.

Jul 14 2017, 6:39 AM · Restricted Project
vladimir.plyashkun added a comment to D34440: [Clang] Expand response files before loading compilation database.

Even if i'll change content of arguments.rsp to
-std=c++11 -Ipath/to/include -Ipath/to/include2 -DMACRO ....
and will try to call clang-tidy process in this way:
clang-tidy -checks=* main.cpp -export-fixes=... -- @arguments.rsp
it also has no effect, because all compiler options will be ignored (i thinks it's because that stripPositionalArgs() function deletes @arguments.rsp parameter as unused input).

Jul 14 2017, 2:38 AM · Restricted Project
vladimir.plyashkun added a comment to D34440: [Clang] Expand response files before loading compilation database.

Thanks @klimek for the response!
Let me give an example.
Suppose we want to check main.cpp by clang-tidy.
As i said before, i cannot use CompilationDatabase.json (due to some technical reasons),
but there is way to pass all options though command line and call external process of clang-tidy in this way:
clang-tidy -checks=* main.cpp -export-fixes=... -- -std=c++11 -Ipath/to/include -Ipath/to/include2 -DMACRO ....
All compiler related options appear in the command line after -- symbol.
But due to command line length restrictions (especially on Windows OS) it's better to use @ResponseFile mechanism.
To achieve it, i've created arguments.rsp file with this content:
-checks=* main.cpp -export-fixes=... -- -std=c++11 -Ipath/to/include -Ipath/to/include2 -DMACRO ....
and tried to call clang-tidy in this way:
clang-tidy @arguments.rsp
But all compiler options are ignored by this call!
If you check how CommonOptionsParser.cpp parses command line arguments, you will see, that first of all, it tries to load fixed compilation database (by finding -- symbol in the command line).
Only after that it calls cl::ParseCommandLineOptions which loads content of arguments.rsp file and parses all the rest arguments.
So, my question was why not to expand response files before loading fixed compilation db from command line?
Or i'm doing something wrong?

Jul 14 2017, 2:23 AM · Restricted Project

Jul 13 2017

vladimir.plyashkun added a comment to D34440: [Clang] Expand response files before loading compilation database.

To discuss:
This patch is required for correct integration Clang-Tidy with CLion IDE.
By this moment, we unable to use CompilationDatabase.json from CLion side which is widely used in Clang-Tidy and in other common tools.
Anyway, there are possibility to pass compiler options directly through command line.
But we cannot pass compiler options directly through command-line, due to command-line-length restrictions.
So, we decided to pass compiler options through @ResponseFile mechanism.
But there are some problems with this approach.
Before this patch, /clang/lib/Tooling/CommonOptionsParser.cpp worked in this way:

  1. Load compilation database from command-line
  2. Expand response files
  3. Parse all other command line arguments

I think it's strange behavior?
Why we try to load compilation database first?
Maybe it's better to expand response files and only after that try to load compilation database and parse another options?
It's hard to refactor cl::ParseCommandLineOptions and cl::ExpandResponseFiles functions, due to high number of usages, so i copied some block directly into /clang/lib/Tooling/CommonOptionsParser.cpp
I don't think that is a best solution, but i don't have another one.

Jul 13 2017, 9:22 AM · Restricted Project
vladimir.plyashkun updated the diff for D34440: [Clang] Expand response files before loading compilation database.
  • trailing period
  • moved test-cases to separate file
Jul 13 2017, 9:05 AM · Restricted Project
vladimir.plyashkun added inline comments to D34440: [Clang] Expand response files before loading compilation database.
Jul 13 2017, 9:00 AM · Restricted Project
vladimir.plyashkun added a comment to D35349: [Clang-Tidy] Preserve Message, FileOffset, FilePath in Clang-Tidy YAML output.

This is second part of review:
https://reviews.llvm.org/D34404
which is contains changes related to clang-tools-extra repository.

Jul 13 2017, 4:53 AM · Restricted Project
vladimir.plyashkun added a comment to D34404: [Clang-Tidy] Preserve Message, FileOffset, FilePath in Clang-Tidy YAML output.

I've splitted single revision into two different revisions:
https://reviews.llvm.org/D34404
https://reviews.llvm.org/D35349
Also, i fixed failing test-cases in clang-apply-replacements due to changes in output format (so they shouldn't be a problem anymore)

Jul 13 2017, 4:37 AM · Restricted Project
vladimir.plyashkun created D35349: [Clang-Tidy] Preserve Message, FileOffset, FilePath in Clang-Tidy YAML output.
Jul 13 2017, 4:34 AM · Restricted Project
vladimir.plyashkun updated the diff for D34404: [Clang-Tidy] Preserve Message, FileOffset, FilePath in Clang-Tidy YAML output.
  1. split revision into two
  2. fix failing tests
Jul 13 2017, 4:31 AM · Restricted Project

Jul 5 2017

vladimir.plyashkun updated the diff for D34404: [Clang-Tidy] Preserve Message, FileOffset, FilePath in Clang-Tidy YAML output.
  • made code less verbose
  • used assignment initialization form
  • removed extra .c_str() call
Jul 5 2017, 9:35 AM · Restricted Project
vladimir.plyashkun added a comment to D34404: [Clang-Tidy] Preserve Message, FileOffset, FilePath in Clang-Tidy YAML output.

Thanks, Alex for your suggestions.
I agree with your remarks, but i thought it's only personal style preferences.
I'll fix it soon.

Jul 5 2017, 7:23 AM · Restricted Project
vladimir.plyashkun added inline comments to D34404: [Clang-Tidy] Preserve Message, FileOffset, FilePath in Clang-Tidy YAML output.
Jul 5 2017, 6:22 AM · Restricted Project
vladimir.plyashkun updated the diff for D34404: [Clang-Tidy] Preserve Message, FileOffset, FilePath in Clang-Tidy YAML output.
  • marked some arguments as const
  • use {} instead of explicit variable declarations
Jul 5 2017, 6:21 AM · Restricted Project
vladimir.plyashkun updated the diff for D34404: [Clang-Tidy] Preserve Message, FileOffset, FilePath in Clang-Tidy YAML output.
  • use EXPECT_* instead of ASSERT_* where it's possible
  • Diagnostic constructor now takes const references for it's arguments
  • removed extra namespace qualifiers
Jul 5 2017, 3:57 AM · Restricted Project

Jul 4 2017

vladimir.plyashkun added inline comments to D34404: [Clang-Tidy] Preserve Message, FileOffset, FilePath in Clang-Tidy YAML output.
Jul 4 2017, 9:37 AM · Restricted Project
vladimir.plyashkun added inline comments to D34404: [Clang-Tidy] Preserve Message, FileOffset, FilePath in Clang-Tidy YAML output.
Jul 4 2017, 9:23 AM · Restricted Project
vladimir.plyashkun updated subscribers of D34404: [Clang-Tidy] Preserve Message, FileOffset, FilePath in Clang-Tidy YAML output.
Jul 4 2017, 4:33 AM · Restricted Project
vladimir.plyashkun updated the diff for D34440: [Clang] Expand response files before loading compilation database.
  • moved test-case from separate file to existing one
  • fixed No newline at end of file problem and put space inside comment
Jul 4 2017, 4:32 AM · Restricted Project

Jul 3 2017

vladimir.plyashkun updated the diff for D34404: [Clang-Tidy] Preserve Message, FileOffset, FilePath in Clang-Tidy YAML output.
  • fixed No newline at end of file problem
  • provided test-case to check that diagnostics with no fixes will be applied correctly
Jul 3 2017, 8:16 AM · Restricted Project

Jun 30 2017

vladimir.plyashkun edited reviewers for D34440: [Clang] Expand response files before loading compilation database, added: ilya-biryukov, alexfh; removed: bkramer, dblaikie.
Jun 30 2017, 2:43 AM · Restricted Project
vladimir.plyashkun edited reviewers for D34404: [Clang-Tidy] Preserve Message, FileOffset, FilePath in Clang-Tidy YAML output, added: ilya-biryukov, alexfh; removed: Restricted Project, fhahn.
Jun 30 2017, 2:43 AM · Restricted Project

Jun 21 2017

vladimir.plyashkun updated the diff for D34404: [Clang-Tidy] Preserve Message, FileOffset, FilePath in Clang-Tidy YAML output.

correct revision with all changes

Jun 21 2017, 4:46 AM · Restricted Project
vladimir.plyashkun updated the diff for D34404: [Clang-Tidy] Preserve Message, FileOffset, FilePath in Clang-Tidy YAML output.

updated CMakeLists.txt

Jun 21 2017, 4:26 AM · Restricted Project
vladimir.plyashkun created D34440: [Clang] Expand response files before loading compilation database.
Jun 21 2017, 3:39 AM · Restricted Project

Jun 20 2017

vladimir.plyashkun retitled D34404: [Clang-Tidy] Preserve Message, FileOffset, FilePath in Clang-Tidy YAML output from Preserve Message, FileOffset, FilePath in Clang-Tidy YAML output to [Clang-Tidy] Preserve Message, FileOffset, FilePath in Clang-Tidy YAML output.
Jun 20 2017, 9:10 AM · Restricted Project
vladimir.plyashkun updated the summary of D34404: [Clang-Tidy] Preserve Message, FileOffset, FilePath in Clang-Tidy YAML output.
Jun 20 2017, 8:58 AM · Restricted Project
vladimir.plyashkun updated the summary of D34404: [Clang-Tidy] Preserve Message, FileOffset, FilePath in Clang-Tidy YAML output.
Jun 20 2017, 8:57 AM · Restricted Project
vladimir.plyashkun created D34404: [Clang-Tidy] Preserve Message, FileOffset, FilePath in Clang-Tidy YAML output.
Jun 20 2017, 8:53 AM · Restricted Project