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.t...@gmail.com
<mailto:jonas.t...@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
<ale...@google.com <mailto:ale...@google.com>> wrote:
On Fri, Oct 6, 2017 at 3:27 PM, Aaron Ballman via cfe-commits
<cfe-commits@lists.llvm.org
<mailto:cfe-commits@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@lists.llvm.org
<mailto:cfe-commits@lists.llvm.org>
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
<http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits>
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
