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

Sat Jun 24 10:52:38 PDT 2017

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?

> 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

>
> 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/20170624/18fb5e8a/attachment-0001.html>