[clang-tools-extra] r315060 - Renaming a test to start with the name of the check based on post-commit review feedback; NFC.
Jonas Toth via cfe-commits
cfe-commits at lists.llvm.org
Thu Oct 12 03:53:09 PDT 2017
Ok.
I subscribe to r315057. I can look at debugging the change as well if
you guys want. Should have time today evening and tomorrow.
Am 12.10.2017 um 12:46 schrieb Alexander Kornienko:
> On Thu, Oct 12, 2017 at 11:12 AM, Jonas Toth <jonas.toth at gmail.com
> <mailto:jonas.toth at gmail.com>> wrote:
>
> Hi,
>
> I am not sure if could follow correctly, but the test for nested
> namespaces comments does not work correctly and is therefore
> currently disabled by fileextension?
>
>
> No, the removal of the file extension was unintentional (likely
> misunderstanding of my post-commit review comment).
>
>
> When running the check from trunk i get the following issues:
>
> $ clang-check google-readability-namespace-comments-cxx17.cpp --
> -std=c++17
> > error: invalid value 'c++17' in '-std=c++17'
>
>
> I suppose, your clang-check is just old. Clang supports -std=c++17
> these days.
>
>
> In the clang-tidy check '-std=c++17' is used. I don't know if the
> c++17 - flag is introduced in clang yet, but maybe this would be
> one issue.
>
> Running clang-tidy from command line does not produce output
> (added fileextension for now):
>
> $ clang-tidy -checks=-*,google-readability-namespace-comments
> google-readability-namespace-comments-cxx17.cpp -- -std=c++1z
> $ clang-tidy -checks=-*,google-readability-namespace-comments
> google-readability-namespace-comments-cxx17.cpp -- -- -std=c++1z
> > Error while processing
> /home/jonas/opt/llvm/tools/clang/tools/extra/test/clang-tidy/google-readability-namespace-comments-cxx17.cpp.
>
> Adding a 'std::cout << "Test if run" << std::endl' at the
> beginning of the `check` - Method works and generates output:
>
> $ clang-tidy -checks=-*,google-readability-namespace-comments
> google-readability-namespace-comments-cxx17.cpp -- -std=c++1z
> Test if run
> Test if run
> Test if run
>
> So I think there is a regression in the Testingcode not catching
> the nested namespaces. Did it work before and what could have changed?
> If it did work and the clang-tidy code is the same this could be a
> regression somewhere else.
>
>
> The problem was that the namespace was shorter than the default
> ShortNamespaceLines option value, so the check didn't trigger. Fixed
> in r315574 (but would be nice, if this kind of stuff was detected
> before commit).
>
> However, this is not the end of problems with this check. It started
> causing assertion failures on some of our code after the recent
> changes (r315057). I'll post an update on that revision once I have
> more details.
>
>
> All the Best, Jonas
>
>
> Am 11.10.2017 um 21:36 schrieb Aaron Ballman:
>
> On Wed, Oct 11, 2017 at 3:29 PM, Alexander Kornienko
> <alexfh at google.com <mailto:alexfh at google.com>> wrote:
>
> On Fri, Oct 6, 2017 at 3:27 PM, Aaron Ballman via cfe-commits
> <cfe-commits at lists.llvm.org
> <mailto:cfe-commits at lists.llvm.org>> wrote:
>
> Author: aaronballman
> Date: Fri Oct 6 06:27:59 2017
> New Revision: 315060
>
> URL:
> http://llvm.org/viewvc/llvm-project?rev=315060&view=rev
> <http://llvm.org/viewvc/llvm-project?rev=315060&view=rev>
> Log:
> Renaming a test to start with the name of the check
> based on post-commit
> review feedback; NFC.
>
> Added:
>
> clang-tools-extra/trunk/test/clang-tidy/google-readability-namespace-comments-cxx17
>
>
> Sorry for not being clear. I didn't mean the `.cpp`
> extension should be
> removed. This effectively disables the test, since lit
> only runs tests in
> files with certain extensions (under
> clang-tools-extra/test these are '.c',
> '.cpp', '.hpp', '.m', '.mm', '.cu', '.ll', '.cl', '.s',
> '.modularize',
> '.module-map-checker', '.test').
>
> That's entirely my fault -- I should have recognized that.
> Sorry for
> the trouble!
>
> I've just renamed the file to *.cpp and the test fails for me:
>
> [0/1] Running the Clang extra tools' regression tests
> FAIL: Clang Tools ::
> clang-tidy/google-readability-namespace-comments-cxx17.cpp
> (102 of 674)
> ******************** TEST 'Clang Tools ::
> clang-tidy/google-readability-namespace-comments-cxx17.cpp'
> FAILED
> ********************
> Script:
> --
> /usr/bin/python2.7
> /src/tools/clang/tools/extra/test/../test/clang-tidy/check_clang_tidy.py
> /src/tools/clang/tools/extra/test/clang-tidy/google-readability-namespace-comments-cxx17.cpp
> google-readability-namespace-comments
> /build/tools/clang/tools/extra/test/clang-tidy/Output/google-readability-namespace-comments-cxx17.cpp.tmp
> -- -- -std=c++17
> --
> Exit Code: 1
>
> Command Output (stdout):
> --
> Running ['clang-tidy',
> '/build/tools/clang/tools/extra/test/clang-tidy/Output/google-readability-namespace-comments-cxx17.cpp.tmp.cpp',
> '-fix',
> '--checks=-*,google-readability-namespace-comments', '--',
> '-std=c++17', '-nostdinc++']...
> ------------------------ clang-tidy output
> -----------------------
>
> ------------------------------------------------------------------
> ------------------------------ Fixes
> -----------------------------
>
> ------------------------------------------------------------------
> FileCheck failed:
> /src/tools/clang/tools/extra/test/clang-tidy/google-readability-namespace-comments-cxx17.cpp:13:17:
> error: expected string not found in input
> // CHECK-FIXES: } // namespace n3
> ^
> /build/tools/clang/tools/extra/test/clang-tidy/Output/google-readability-namespace-comments-cxx17.cpp.tmp.cpp:1:1:
> note: scanning from here
> // RUN: %check_clang_tidy %s
> google-readability-namespace-comments %t -- --
> -std=c++17
> ^
> /build/tools/clang/tools/extra/test/clang-tidy/Output/google-readability-namespace-comments-cxx17.cpp.tmp.cpp:5:7:
> note: possible intended match here
> // So that namespace is not empty.
> ^
>
>
> --
> Command Output (stderr):
> --
> Traceback (most recent call last):
> File
> "/src/tools/clang/tools/extra/test/../test/clang-tidy/check_clang_tidy.py",
> line 140, in <module>
> main()
> File
> "/src/tools/clang/tools/extra/test/../test/clang-tidy/check_clang_tidy.py",
> line 121, in main
> stderr=subprocess.STDOUT)
> File "/usr/lib/python2.7/subprocess.py", line 573, in
> check_output
> raise CalledProcessError(retcode, cmd, output=output)
> subprocess.CalledProcessError: Command '['FileCheck',
> '-input-file=/build/tools/clang/tools/extra/test/clang-tidy/Output/google-readability-namespace-comments-cxx17.cpp.tmp.cpp',
> '/src/tools/clang/tools/extra/test/clang-tidy/google-readability-namespace-comments-cxx17.cpp',
> '-check-prefix=CHECK-FIXES', '-strict-whitespace']'
> returned non-zero exit
> status 1
>
> --
>
> ********************
> Testing Time: 13.07s
> ********************
> Failing Tests (1):
> Clang Tools ::
> clang-tidy/google-readability-namespace-comments-cxx17.cpp
>
> Expected Passes : 673
> Unexpected Failures: 1
> FAILED:
> tools/clang/tools/extra/test/CMakeFiles/check-clang-tools
>
>
> Did you experience anything similar? Any ideas?
>
> I now get the same behavior that you're seeing. I'm not
> certain what's
> going on there (and don't have the time to look into it at the
> moment), but perhaps Jonas has ideas.
>
> ~Aaron
>
> - copied unchanged from r315059,
> clang-tools-extra/trunk/test/clang-tidy/google-readability-nested-namespace-comments.cpp
> Removed:
>
> clang-tools-extra/trunk/test/clang-tidy/google-readability-nested-namespace-comments.cpp
>
> Removed:
> clang-tools-extra/trunk/test/clang-tidy/google-readability-nested-namespace-comments.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clang-tidy/google-readability-nested-namespace-comments.cpp?rev=315059&view=auto
> <http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clang-tidy/google-readability-nested-namespace-comments.cpp?rev=315059&view=auto>
>
> ==============================================================================
> ---
> clang-tools-extra/trunk/test/clang-tidy/google-readability-nested-namespace-comments.cpp
> (original)
> +++
> clang-tools-extra/trunk/test/clang-tidy/google-readability-nested-namespace-comments.cpp
> (removed)
> @@ -1,15 +0,0 @@
> -// RUN: %check_clang_tidy %s
> google-readability-namespace-comments %t --
> -- -std=c++17
> -
> -namespace n1::n2 {
> -namespace n3 {
> - // So that namespace is not empty.
> - void f();
> -
> -// CHECK-MESSAGES: :[[@LINE+4]]:2: warning: namespace
> 'n3' not terminated
> with
> -// CHECK-MESSAGES: :[[@LINE-7]]:11: note: namespace
> 'n3' starts here
> -// CHECK-MESSAGES: :[[@LINE+2]]:3: warning: namespace
> 'n1::n2' not
> terminated with a closing comment
> [google-readability-namespace-comments]
> -// CHECK-MESSAGES: :[[@LINE-10]]:11: note: namespace
> 'n1::n2' starts here
> -}}
> -// CHECK-FIXES: } // namespace n3
> -// CHECK-FIXES: } // namespace n1::n2
> -
>
>
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at lists.llvm.org
> <mailto:cfe-commits at lists.llvm.org>
> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
> <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/20171012/8e09baa0/attachment-0001.html>
More information about the cfe-commits
mailing list