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
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:
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
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
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
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
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.
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,
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
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.
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
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
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=*
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,
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
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);
+
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
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
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
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
24 matches
Mail list logo
