Done in r307661. On Mon, Jul 10, 2017 at 2:08 PM, Alexander Kornienko <ale...@google.com> wrote:
> 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.htm >>>>>>>>>> l#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::loadFromCommandLin >>>>>>>>>>>>>>> e >>>>>>>>>>>>>>> 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/tool >>>>>>>>>>>>>>>> s/extra/test/clang-tidy/diagnostic.cpp.nonexistent.cpp. >>>>>>>>>>>>>>>> /usr/local/google/home/blaikie >>>>>>>>>>>>>>>> /dev/llvm/src/tools/clang/tool >>>>>>>>>>>>>>>> s/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/tool >>>>>>>>>>>>>>>> s/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/tool >>>>>>>>>>>>>>>> s/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/tool >>>>>>>>>>>>>>>> s/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/tool >>>>>>>>>>>>>>>> s/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
