[clang-tools-extra] r303735 - Modify test so that it looks for patterns in stderr as well

Wed Jul 12 07:46:29 PDT 2017

Done in r307661.

On Mon, Jul 10, 2017 at 2:08 PM, Alexander Kornienko <alexfh at 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 <alexfh at 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 <dblaikie at gmail.com>
>> wrote:
>>
>>>
>>>
>>> On Mon, Jun 26, 2017 at 5:31 AM Serge Pavlov <sepavloff at gmail.com>
>>> wrote:
>>>
>>>> 2017-06-26 4:05 GMT+07:00 David Blaikie <dblaikie at 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 <sepavloff at gmail.com>
>>>>> wrote:
>>>>>
>>>>>> 2017-06-25 0:52 GMT+07:00 David Blaikie <dblaikie at gmail.com>:
>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> On Sat, Jun 24, 2017 at 10:08 AM Serge Pavlov <sepavloff at 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 <dblaikie at 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 <dblaikie at 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 <sepavloff at gmail.com>
>>>>>>>>>> wrote:
>>>>>>>>>>
>>>>>>>>>>> 2017-06-15 2:43 GMT+07:00 David Blaikie <dblaikie at gmail.com>:
>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>> On Wed, Jun 14, 2017, 8:17 AM Serge Pavlov <sepavloff at gmail.com>
>>>>>>>>>>>> wrote:
>>>>>>>>>>>>
>>>>>>>>>>>>> 2017-06-14 4:24 GMT+07:00 David Blaikie <dblaikie at 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 <
>>>>>>>>>>>>>> sepavloff at 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 <dblaikie at 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 at 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 at lists.llvm.org
>>>>>>>>>>>>>>>>> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>
>>>>>>>>
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20170712/41db654d/attachment-0001.html>