Sorry, missed this thread somehow. So, the behavior itself seems incorrect. Clang tools should not be trying to find a compilation database in case the command line has a `--`, but the driver fails to parse it. It should be a hard failure. We also need a reliable test for this behavior (with a compile_commands.json being put into a test directory or generated during a test).
On Tue, Jun 27, 2017 at 3:33 AM, David Blaikie <dblai...@gmail.com> wrote: > > > On Mon, Jun 26, 2017 at 5:31 AM Serge Pavlov <sepavl...@gmail.com> wrote: > >> 2017-06-26 4:05 GMT+07:00 David Blaikie <dblai...@gmail.com>: >> >>> Ah, I see now then. >>> >>> I have a symlink from the root of my source directory pointing to the >>> compile_commands.json in my build directory. >>> >>> I have this so that the vim YouCompleteMe plugin (& any other clang >>> tools) can find it, as they usually should, for using tools with the >>> llvm/clang project... >>> >>> Sounds like this test is incompatible with using the tooling >>> infrastructure in the llvm/clang project? >>> >> Any test that relies on compilation database search can fail in such >> case. Maybe the root of the tools test could contain some special >> compile_commands.json so that the search will always end up in definite >> state? >> > > Perhaps - or maybe tools could have a parameter limiting how many parent > directories to recurse up through? But yeah, dunno what the best solution > would be. > > >> >> >>> >>> On Sun, Jun 25, 2017, 10:24 AM Serge Pavlov <sepavl...@gmail.com> wrote: >>> >>>> 2017-06-25 0:52 GMT+07:00 David Blaikie <dblai...@gmail.com>: >>>> >>>>> >>>>> >>>>> On Sat, Jun 24, 2017 at 10:08 AM Serge Pavlov <sepavl...@gmail.com> >>>>> wrote: >>>>> >>>>>> With CMAKE_EXPORT_COMPILE_COMMANDS the file compile_commands.json is >>>>>> created in the directory >>>>>> <build-dir>/tools/clang/tools/extra/test/clang-tidy/Output, >>>>>> >>>>>> >>>>> >>>>> I'd be really surprised if this is the case - why would >>>>> cmake/ninja/makefiles put the compile commands for the whole LLVM >>>>> project/build in that somewhat random subdirectory? >>>>> >>>> >>>> I was wrong, these json files were not created by cmake run but appear >>>> during test run. The file created by cmake is in the build root. >>>> >>>> >>>>> >>>>> >>>>>> but the tests from <src-dir>/llvm/tools/clang/tools/extra/test/clang-tidy >>>>>> run in the directory <build-dir>/tools/clang/tools/extra/test/clang-tidy, >>>>>> which does not contain json files. So the test passes successfully. >>>>>> Ubuntu >>>>>> 16.04, cmake 3.5.1. >>>>>> >>>>> >>>>> Ah, perhaps you found a compile_commands for one of the test cases, >>>>> not the one generated by CMake. CMake 3.5.1 doesn't support >>>>> CMAKE_EXPORT_COMPILE_COMMANDS. >>>>> >>>>> It was added in 3.5.2, according to the documentation: https://cmake. >>>>> org/cmake/help/v3.5/variable/CMAKE_EXPORT_COMPILE_COMMANDS.html >>>>> >>>>> >>>> >>>> It was added in 2.8.5 according to documentation ( >>>> http://clang.llvm.org/docs/JSONCompilationDatabase.html# >>>> supported-systems), at least the version 3.5.1 creates compilation >>>> databases. >>>> >>>> clang-tidy tries to create compilation database from source path, >>>> looking for compile_commands.json in the directory where provided source >>>> file resides and in all its parent directories. If source tree is in a >>>> subdirectory of build tree, then compile_commands.json in the build >>>> directory would be found and the test would fail. Is it your case? >>>> >>>> >>>>>> Thanks, >>>>>> --Serge >>>>>> >>>>>> 2017-06-24 9:42 GMT+07:00 David Blaikie <dblai...@gmail.com>: >>>>>> >>>>>>> Ping (+Manuel, perhaps he's got some ideas about this, given >>>>>>> background in the tooling & compilation database work, or could point >>>>>>> this >>>>>>> to someone who does?) >>>>>>> >>>>>>> >>>>>>> On Thu, Jun 15, 2017 at 10:40 AM David Blaikie <dblai...@gmail.com> >>>>>>> wrote: >>>>>>> >>>>>>>> https://sarcasm.github.io/notes/dev/compilation-database.html#cmake >>>>>>>> >>>>>>>> If you enable the CMAKE_EXPORT_COMPILE_COMMANDS option in cmake (& >>>>>>>> have a sufficiently recent cmake), then CMake will generate a >>>>>>>> compile_commands.json in the root of the build tree. The test finds >>>>>>>> this & >>>>>>>> fails, instead of finding no compilation database & succeeding. >>>>>>>> >>>>>>>> (to use this, you can then symlink from the root of the source tree >>>>>>>> to point to this in your build tree - this is how I get YCM to work >>>>>>>> for my >>>>>>>> LLVM builds & could work for other clang tools as well) >>>>>>>> >>>>>>>> On Thu, Jun 15, 2017 at 7:51 AM Serge Pavlov <sepavl...@gmail.com> >>>>>>>> wrote: >>>>>>>> >>>>>>>>> 2017-06-15 2:43 GMT+07:00 David Blaikie <dblai...@gmail.com>: >>>>>>>>> >>>>>>>>>> >>>>>>>>>> >>>>>>>>>> On Wed, Jun 14, 2017, 8:17 AM Serge Pavlov <sepavl...@gmail.com> >>>>>>>>>> wrote: >>>>>>>>>> >>>>>>>>>>> 2017-06-14 4:24 GMT+07:00 David Blaikie <dblai...@gmail.com>: >>>>>>>>>>> >>>>>>>>>>>> Ah, I find that the test passes if I remove the >>>>>>>>>>>> compile_commands.json file from my build directory (I have Ninja >>>>>>>>>>>> configured >>>>>>>>>>>> to generate a compile_commands.json file). >>>>>>>>>>>> >>>>>>>>>>>> Looks like what happens is it finds the compilation database >>>>>>>>>>>> and fails hard when the database doesn't contain a compile command >>>>>>>>>>>> for the >>>>>>>>>>>> file in question. If the database is not found, it falls back to >>>>>>>>>>>> some basic >>>>>>>>>>>> command behavior, perhaps? >>>>>>>>>>>> >>>>>>>>>>>> >>>>>>>>>>> You are right, constructor of `CommonOptionsParser` calls >>>>>>>>>>> `autoDetectFromSource` or `autoDetectFromDirectory` prior to final >>>>>>>>>>> construction of `FixedCompilationDatabase. >>>>>>>>>>> >>>>>>>>>>> Is there some way this test could be fixed to cope with this, >>>>>>>>>>>> otherwise it seems to get in the way of people actually using >>>>>>>>>>>> clang tools >>>>>>>>>>>> in their LLVM/Clang build environment? >>>>>>>>>>>> >>>>>>>>>>>> >>>>>>>>>>> IIUC, presence of stale compilation database file in test >>>>>>>>>>> directory could break many tests. I don't understand why only >>>>>>>>>>> diagnostic.cpp fails, probably there is something wrong with the >>>>>>>>>>> clang-tidy >>>>>>>>>>> application cleanup in this case? >>>>>>>>>>> >>>>>>>>>> >>>>>>>>>> Except it's neither stale nor in the test directory. >>>>>>>>>> >>>>>>>>>> It's the up to date/useful/used compile_commands.json generated >>>>>>>>>> by ninja in the root of the build tree. >>>>>>>>>> >>>>>>>>> >>>>>>>>> I miss something. If I could reproduce the problem, I would >>>>>>>>> investigate it. >>>>>>>>> >>>>>>>>> >>>>>>>>>> >>>>>>>>>> >>>>>>>>>>> >>>>>>>>>>>> On Tue, Jun 13, 2017 at 7:41 AM Serge Pavlov < >>>>>>>>>>>> sepavl...@gmail.com> wrote: >>>>>>>>>>>> >>>>>>>>>>>>> I cannot reproduce such fail, so I can only guess how changes >>>>>>>>>>>>> made in https://reviews.llvm.org/rL303756 and >>>>>>>>>>>>> https://reviews.llvm.org/rL303741 could cause such problem. >>>>>>>>>>>>> Behavior of `Driver::BuildCompilation` is changed so that it >>>>>>>>>>>>> returns null >>>>>>>>>>>>> pointer if errors occur during driver argument parse. It is >>>>>>>>>>>>> called in >>>>>>>>>>>>> `CompilationDatabase.cpp` from `stripPositionalArgs`. The call >>>>>>>>>>>>> stack at >>>>>>>>>>>>> this point is: >>>>>>>>>>>>> stripPositionalArgs >>>>>>>>>>>>> clang::tooling::FixedCompilationDatabase::loadFromCommandLine >>>>>>>>>>>>> clang::tooling::CommonOptionsParser::CommonOptionsParser >>>>>>>>>>>>> clang::tidy::clangTidyMain >>>>>>>>>>>>> main >>>>>>>>>>>>> `FixedCompilationDatabase::loadFromCommandLine` returns null >>>>>>>>>>>>> and CommonOptionsParser uses another method to create compilation >>>>>>>>>>>>> database. >>>>>>>>>>>>> The output "Compile command not found" means that no input file >>>>>>>>>>>>> were found >>>>>>>>>>>>> in `ClangTool::run`. Maybe some file names are nulls? >>>>>>>>>>>>> >>>>>>>>>>>>> >>>>>>>>>>>>> Thanks, >>>>>>>>>>>>> --Serge >>>>>>>>>>>>> >>>>>>>>>>>>> 2017-06-13 3:42 GMT+07:00 David Blaikie <dblai...@gmail.com>: >>>>>>>>>>>>> >>>>>>>>>>>>>> I've been seeing errors from this test recently: >>>>>>>>>>>>>> >>>>>>>>>>>>>> Command Output (stderr): >>>>>>>>>>>>>> -- >>>>>>>>>>>>>> 1 error generated. >>>>>>>>>>>>>> Error while processing /usr/local/google/home/ >>>>>>>>>>>>>> blaikie/dev/llvm/src/tools/clang/tools/extra/test/clang- >>>>>>>>>>>>>> tidy/diagnostic.cpp.nonexistent.cpp. >>>>>>>>>>>>>> /usr/local/google/home/blaikie/dev/llvm/src/tools/ >>>>>>>>>>>>>> clang/tools/extra/test/clang-tidy/diagnostic.cpp:10:12: >>>>>>>>>>>>>> error: expected string not found in input >>>>>>>>>>>>>> // CHECK2: :[[@LINE+2]]:9: warning: implicit conversion from >>>>>>>>>>>>>> 'double' to 'int' changes value from 1.5 to 1 >>>>>>>>>>>>>> [clang-diagnostic-literal- >>>>>>>>>>>>>> conversion] >>>>>>>>>>>>>> ^ >>>>>>>>>>>>>> <stdin>:2:1: note: scanning from here >>>>>>>>>>>>>> Skipping /usr/local/google/home/blaikie/dev/llvm/src/tools/ >>>>>>>>>>>>>> clang/tools/extra/test/clang-tidy/diagnostic.cpp. Compile >>>>>>>>>>>>>> command not found. >>>>>>>>>>>>>> ^ >>>>>>>>>>>>>> <stdin>:2:1: note: with expression "@LINE+2" equal to "12" >>>>>>>>>>>>>> Skipping /usr/local/google/home/blaikie/dev/llvm/src/tools/ >>>>>>>>>>>>>> clang/tools/extra/test/clang-tidy/diagnostic.cpp. Compile >>>>>>>>>>>>>> command not found. >>>>>>>>>>>>>> ^ >>>>>>>>>>>>>> >>>>>>>>>>>>>> >>>>>>>>>>>>>> Specifically, the output is: >>>>>>>>>>>>>> $ ./bin/clang-tidy >>>>>>>>>>>>>> -checks='-*,clang-diagnostic-*,google-explicit-constructor' >>>>>>>>>>>>>> /usr/local/google/home/blaikie/dev/llvm/src/tools/ >>>>>>>>>>>>>> clang/tools/extra/test/clang-tidy/diagnostic.cpp -- >>>>>>>>>>>>>> -fan-unknown-option 2>&1 error: >>>>>>>>>>>>>> unknown >>>>>>>>>>>>>> argument: '-fan-unknown-option' >>>>>>>>>>>>>> Skipping >>>>>>>>>>>>>> /usr/local/google/home/blaikie/dev/llvm/src/tools/ >>>>>>>>>>>>>> clang/tools/extra/test/clang-tidy/diagnostic.cpp. Compile >>>>>>>>>>>>>> command not found. >>>>>>>>>>>>>> >>>>>>>>>>>>>> >>>>>>>>>>>>>> Does this look like it might be related to any of your >>>>>>>>>>>>>> changes in this area? Perhaps the error due to unknown argument >>>>>>>>>>>>>> is causing >>>>>>>>>>>>>> clang-tidy not to continue on to run the check & report the >>>>>>>>>>>>>> warning? >>>>>>>>>>>>>> >>>>>>>>>>>>>> >>>>>>>>>>>>>> On Wed, May 24, 2017 at 3:51 AM Serge Pavlov via cfe-commits < >>>>>>>>>>>>>> cfe-commits@lists.llvm.org> wrote: >>>>>>>>>>>>>> >>>>>>>>>>>>>>> Author: sepavloff >>>>>>>>>>>>>>> Date: Wed May 24 05:50:56 2017 >>>>>>>>>>>>>>> New Revision: 303735 >>>>>>>>>>>>>>> >>>>>>>>>>>>>>> URL: http://llvm.org/viewvc/llvm-project?rev=303735&view=rev >>>>>>>>>>>>>>> Log: >>>>>>>>>>>>>>> Modify test so that it looks for patterns in stderr as well >>>>>>>>>>>>>>> >>>>>>>>>>>>>>> With the change https://reviews.llvm.org/D33013 driver will >>>>>>>>>>>>>>> not build >>>>>>>>>>>>>>> compilation object if command line is invalid, in >>>>>>>>>>>>>>> particular, if >>>>>>>>>>>>>>> unrecognized option is provided. In such cases it will >>>>>>>>>>>>>>> prints diagnostics >>>>>>>>>>>>>>> on stderr. The test 'clang-tidy/diagnostic.cpp' checks >>>>>>>>>>>>>>> reaction on >>>>>>>>>>>>>>> unrecognized option and will fail when D33013 is applied >>>>>>>>>>>>>>> because it checks >>>>>>>>>>>>>>> only stdout for test patterns and expects the name of >>>>>>>>>>>>>>> diagnostic category >>>>>>>>>>>>>>> prepared by clang-tidy. With this change the test makes more >>>>>>>>>>>>>>> general check >>>>>>>>>>>>>>> and must work in either case. >>>>>>>>>>>>>>> >>>>>>>>>>>>>>> Differential Revision: https://reviews.llvm.org/D33173 >>>>>>>>>>>>>>> >>>>>>>>>>>>>>> Modified: >>>>>>>>>>>>>>> clang-tools-extra/trunk/test/clang-tidy/diagnostic.cpp >>>>>>>>>>>>>>> >>>>>>>>>>>>>>> Modified: clang-tools-extra/trunk/test/ >>>>>>>>>>>>>>> clang-tidy/diagnostic.cpp >>>>>>>>>>>>>>> URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/ >>>>>>>>>>>>>>> trunk/test/clang-tidy/diagnostic.cpp?rev=303735&r1= >>>>>>>>>>>>>>> 303734&r2=303735&view=diff >>>>>>>>>>>>>>> ============================================================ >>>>>>>>>>>>>>> ================== >>>>>>>>>>>>>>> --- clang-tools-extra/trunk/test/clang-tidy/diagnostic.cpp >>>>>>>>>>>>>>> (original) >>>>>>>>>>>>>>> +++ clang-tools-extra/trunk/test/clang-tidy/diagnostic.cpp >>>>>>>>>>>>>>> Wed May 24 05:50:56 2017 >>>>>>>>>>>>>>> @@ -1,11 +1,11 @@ >>>>>>>>>>>>>>> // RUN: clang-tidy -checks='-*,modernize-use-override' >>>>>>>>>>>>>>> %s.nonexistent.cpp -- | FileCheck -check-prefix=CHECK1 >>>>>>>>>>>>>>> -implicit-check-not='{{warning:|error:}}' %s >>>>>>>>>>>>>>> -// RUN: clang-tidy >>>>>>>>>>>>>>> -checks='-*,clang-diagnostic-*,google-explicit-constructor' >>>>>>>>>>>>>>> %s -- -fan-unknown-option | FileCheck -check-prefix=CHECK2 >>>>>>>>>>>>>>> -implicit-check-not='{{warning:|error:}}' %s >>>>>>>>>>>>>>> -// RUN: clang-tidy -checks='-*,google-explicit- >>>>>>>>>>>>>>> constructor,clang-diagnostic-literal-conversion' %s -- >>>>>>>>>>>>>>> -fan-unknown-option | FileCheck -check-prefix=CHECK3 >>>>>>>>>>>>>>> -implicit-check-not='{{warning:|error:}}' >>>>>>>>>>>>>>> %s >>>>>>>>>>>>>>> +// RUN: clang-tidy >>>>>>>>>>>>>>> -checks='-*,clang-diagnostic-*,google-explicit-constructor' >>>>>>>>>>>>>>> %s -- -fan-unknown-option 2>&1 | FileCheck -check-prefix=CHECK2 >>>>>>>>>>>>>>> -implicit-check-not='{{warning:|error:}}' %s >>>>>>>>>>>>>>> +// RUN: clang-tidy -checks='-*,google-explicit- >>>>>>>>>>>>>>> constructor,clang-diagnostic-literal-conversion' %s -- >>>>>>>>>>>>>>> -fan-unknown-option 2>&1 | FileCheck -check-prefix=CHECK3 >>>>>>>>>>>>>>> -implicit-check-not='{{warning:|error:}}' %s >>>>>>>>>>>>>>> // RUN: clang-tidy -checks='-*,modernize-use- >>>>>>>>>>>>>>> override,clang-diagnostic-macro-redefined' %s -- >>>>>>>>>>>>>>> -DMACRO_FROM_COMMAND_LINE | FileCheck -check-prefix=CHECK4 >>>>>>>>>>>>>>> -implicit-check-not='{{warning:|error:}}' %s >>>>>>>>>>>>>>> >>>>>>>>>>>>>>> // CHECK1: error: error reading '{{.*}}.nonexistent.cpp' >>>>>>>>>>>>>>> [clang-diagnostic-error] >>>>>>>>>>>>>>> -// CHECK2: error: unknown argument: '-fan-unknown-option' >>>>>>>>>>>>>>> [clang-diagnostic-error] >>>>>>>>>>>>>>> -// CHECK3: error: unknown argument: '-fan-unknown-option' >>>>>>>>>>>>>>> [clang-diagnostic-error] >>>>>>>>>>>>>>> +// CHECK2: error: unknown argument: '-fan-unknown-option' >>>>>>>>>>>>>>> +// CHECK3: error: unknown argument: '-fan-unknown-option' >>>>>>>>>>>>>>> >>>>>>>>>>>>>>> // CHECK2: :[[@LINE+2]]:9: warning: implicit conversion >>>>>>>>>>>>>>> from 'double' to 'int' changes value from 1.5 to 1 >>>>>>>>>>>>>>> [clang-diagnostic-literal-conversion] >>>>>>>>>>>>>>> // CHECK3: :[[@LINE+1]]:9: warning: implicit conversion >>>>>>>>>>>>>>> from 'double' to 'int' changes value >>>>>>>>>>>>>>> >>>>>>>>>>>>>>> >>>>>>>>>>>>>>> _______________________________________________ >>>>>>>>>>>>>>> cfe-commits mailing list >>>>>>>>>>>>>>> cfe-commits@lists.llvm.org >>>>>>>>>>>>>>> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits >>>>>>>>>>>>>>> >>>>>>>>>>>>>> >>>>>>>>>>>>> >>>>>>
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits