Thanks all! On Wed, Jul 12, 2017 at 7:46 AM Alexander Kornienko <ale...@google.com> wrote:
> 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/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