User Details
- User Since
- Jun 20 2017, 4:10 AM (294 w, 8 h)
Aug 17 2022
Aug 16 2022
- return operand source range too
Aug 15 2022
- Executed clang-format after changes
- Return operator location (in additional range)
Aug 11 2022
Hi @carlosgalvezp
Yes, sorry, maybe screenshot and example from issue in our bugtracker is not the best one.
You can check it on this sample from clang-tidy tests:
The "hicpp-signed-bitwise" will point to
Aug 10 2022
Oct 23 2019
Oct 15 2019
Oct 11 2019
@aaron.ballman
@JonasToth
Thanks, i agree too! I've prepared new revision with additional option to preserve old inspection behaviour.
Provide additional option to preserve current inspection behaviour
Oct 9 2019
Jan 22 2018
Thanks, now all should be fine!
Jan 19 2018
Some more nits were fixed.
Fixed code review remarks.
Fixed comment
Jan 18 2018
Moved logic to ClangTidyMain
Implemented test-case to check that BaseFS is actually used in ClangTool
Jan 17 2018
Friendly ping
Jan 11 2018
Second part here (provided default virtual filesystem argument to ClangTool constructor): https://reviews.llvm.org/D41947
Dec 28 2017
Dec 27 2017
FYI, i've create revision to support ivfsoverlay option in Tooling: https://reviews.llvm.org/D41594
Dec 24 2017
@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 21 2017
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:
Aug 16 2017
@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
Jul 31 2017
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 18 2017
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:
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 14 2017
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.
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).
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 13 2017
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:
- Load compilation database from command-line
- Expand response files
- 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.
- trailing period
- moved test-cases to separate file
This is second part of review:
https://reviews.llvm.org/D34404
which is contains changes related to clang-tools-extra repository.
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)
- split revision into two
- fix failing tests
Jul 5 2017
- made code less verbose
- used assignment initialization form
- removed extra .c_str() call
Thanks, Alex for your suggestions.
I agree with your remarks, but i thought it's only personal style preferences.
I'll fix it soon.
- marked some arguments as const
- use {} instead of explicit variable declarations
- use EXPECT_* instead of ASSERT_* where it's possible
- Diagnostic constructor now takes const references for it's arguments
- removed extra namespace qualifiers
Jul 4 2017
- moved test-case from separate file to existing one
- fixed No newline at end of file problem and put space inside comment
Jul 3 2017
- fixed No newline at end of file problem
- provided test-case to check that diagnostics with no fixes will be applied correctly
Jun 30 2017
Jun 21 2017
correct revision with all changes
updated CMakeLists.txt