chh added a comment.
Herald added a subscriber: llvm-commits.
FYI, Android NDK has another use case in
https://github.com/android-ndk/ndk/issues/680.
It would be nice to have clang-tidy recognize the response file.
Repository:
rL LLVM
https://reviews.llvm.org/D34440
__
alexfh added a comment.
In https://reviews.llvm.org/D34440#843192, @vladimir.plyashkun wrote:
> @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
vladimir.plyashkun added a comment.
@alexf
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
alexfh added a comment.
In https://reviews.llvm.org/D34440#825924, @vladimir.plyashkun wrote:
> > 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
vladimir.plyashkun added a comment.
> 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
Repository:
rL LLVM
https://re
asl added a comment.
In https://reviews.llvm.org/D34440#809525, @alexfh wrote:
> In https://reviews.llvm.org/D34440#808156, @vladimir.plyashkun wrote:
>
> > **To discuss:**
> > ...
> > By this moment, we unable to use //CompilationDatabase.json// from //CLion
> > //side which is widely used in
vladimir.plyashkun added a comment.
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 c
klimek added a comment.
In https://reviews.llvm.org/D34440#812938, @vladimir.plyashkun wrote:
> > If you want one unified format, the compilation database is it.
>
> Is the `clang-tidy ... -- ` meant to be more or less a drop-in
> replacement for `clang ` (arguments-wise)?
> If yes, this expan
vladimir.plyashkun added a comment.
> If you want one unified format, the compilation database is it.
Is the `clang-tidy ... -- ` meant to be more or less a drop-in
replacement for `clang ` (arguments-wise)?
If yes, this expansion of response files here is an another step in this
direction.
O
On Fri, Jul 14, 2017 at 02:40:25PM +, Manuel Klimek via Phabricator via
cfe-commits wrote:
> klimek added a comment.
>
> In https://reviews.llvm.org/D34440#809581, @vladimir.plyashkun wrote:
>
> > > Are there any concerns using the alternative?
> >
> > I can't say that it's a big problems, b
klimek added a comment.
In https://reviews.llvm.org/D34440#809581, @vladimir.plyashkun wrote:
> > 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 au
vladimir.plyashkun added a comment.
> 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
alexfh added a comment.
In https://reviews.llvm.org/D34440#809522, @klimek wrote:
> In https://reviews.llvm.org/D34440#809325, @vladimir.plyashkun wrote:
>
> > Even if i'll change content of //arguments.rsp// to
> > `-std=c++11 -Ipath/to/include -Ipath/to/include2 -DMACRO `
> > and will try
alexfh added a comment.
In https://reviews.llvm.org/D34440#808156, @vladimir.plyashkun wrote:
> **To discuss:**
> ...
> By this moment, we unable to use //CompilationDatabase.json// from //CLion
> //side which is widely used in //Clang-Tidy// and in other common tools.
It would be interestin
klimek added a comment.
In https://reviews.llvm.org/D34440#809325, @vladimir.plyashkun wrote:
> 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=* ma
vladimir.plyashkun added a comment.
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, bec
vladimir.plyashkun added a comment.
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
klimek added a comment.
In https://reviews.llvm.org/D34440#808156, @vladimir.plyashkun wrote:
> **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
vladimir.plyashkun added a comment.
**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
vladimir.plyashkun updated this revision to Diff 106436.
vladimir.plyashkun added a comment.
- trailing period
- moved test-cases to separate file
Repository:
rL LLVM
https://reviews.llvm.org/D34440
Files:
lib/Tooling/CommonOptionsParser.cpp
unittests/Tooling/CMakeLists.txt
unittests/T
vladimir.plyashkun added inline comments.
Comment at: unittests/Tooling/CompilationDatabaseTest.cpp:638-652
+ ParseCompilationDatabaseFromResponseFileTest() {
+std::error_code EC = llvm::sys::fs::createUniqueDirectory("unittest",
TestDir);
+EXPECT_TRUE(!EC);
+llvm::
alexfh requested changes to this revision.
alexfh added inline comments.
This revision now requires changes to proceed.
Comment at: lib/Tooling/CommonOptionsParser.cpp:120
+
+ // Expand response files before loading compilation database from command
line
+ SmallVector newArg
vladimir.plyashkun updated this revision to Diff 105153.
vladimir.plyashkun added a subscriber: alexfh.
vladimir.plyashkun added a comment.
- moved test-case from separate file to existing one
- fixed `No newline at end of file` problem and put space inside comment
Repository:
rL LLVM
https:/
alexfh requested changes to this revision.
alexfh added a comment.
This revision now requires changes to proceed.
Please re-upload the patch with full context (see
http://llvm.org/docs/Phabricator.html).
Comment at: lib/Tooling/CommonOptionsParser.cpp:119
cl::HideUnrelatedO
vladimir.plyashkun created this revision.
vladimir.plyashkun added a project: clang.
Herald added a subscriber: mgorny.
Due to command line length restrictions, arguments can be passed through
response files.
Before trying to load compilation database from command line, response files
should be
25 matches
Mail list logo
