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

Alexander Kornienko via cfe-commits cfe-commits at lists.llvm.org
Mon Jul 10 02:47:57 PDT 2017 

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/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 <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.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 <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::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 <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/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 at 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 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/20170710/4df75ab8/attachment-0001.html>

More information about the cfe-commits mailing list