Benjamin has actually fixed the issue by reverting to the old behavior in r306822. I'll add a test for this behavior anyway.
On Mon, Jul 10, 2017 at 11:47 AM, Alexander Kornienko <ale...@google.com> wrote: > 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/cl >>>>>>> ang/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/CM >>>>>> AKE_EXPORT_COMPILE_COMMANDS.html >>>>>> >>>>>> >>>>> >>>>> It was added in 2.8.5 according to documentation ( >>>>> http://clang.llvm.org/docs/JSONCompilationDatabase.html#sup >>>>> ported-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-conv >>>>>>>>>>>>>>> ersion] >>>>>>>>>>>>>>> ^ >>>>>>>>>>>>>>> <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-pr >>>>>>>>>>>>>>>> oject?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/c >>>>>>>>>>>>>>>> lang-tidy/diagnostic.cpp >>>>>>>>>>>>>>>> URL: http://llvm.org/viewvc/llvm-pr >>>>>>>>>>>>>>>> oject/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-co >>>>>>>>>>>>>>>> nstructor,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-co >>>>>>>>>>>>>>>> nstructor,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-over >>>>>>>>>>>>>>>> ride,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
