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

David Blaikie via cfe-commits cfe-commits at lists.llvm.org
Wed Jul 12 07:51:13 PDT 2017 

Thanks all!

On Wed, Jul 12, 2017 at 7:46 AM Alexander Kornienko <alexfh at google.com>
wrote:

> 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/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/20170712/f70bc078/attachment-0001.html>

More information about the cfe-commits mailing list