<html>
<head>
<meta http-equiv="Content-Type" content="text/html; charset=utf-8">
</head>
<body text="#000000" bgcolor="#FFFFFF">
<p>Ok.</p>
<p>I subscribe to r315057. I can look at debugging the change as
well if you guys want. Should have time today evening and
tomorrow.<br>
</p>
<br>
<div class="moz-cite-prefix">Am 12.10.2017 um 12:46 schrieb
Alexander Kornienko:<br>
</div>
<blockquote type="cite"
cite="mid:CAOweq9KSQx-zOjOoAtOeEYxP3dD+fWcGWpo7-So=9p1m3GNwdQ@mail.gmail.com">
<div dir="ltr">
<div class="gmail_extra">
<div class="gmail_quote">On Thu, Oct 12, 2017 at 11:12 AM,
Jonas Toth <span dir="ltr"><<a
href="mailto:jonas.toth@gmail.com" target="_blank"
moz-do-not-send="true">jonas.toth@gmail.com</a>></span>
wrote:<br>
<blockquote class="gmail_quote" style="margin:0px 0px 0px
0.8ex;border-left:1px solid
rgb(204,204,204);padding-left:1ex">Hi,<br>
<br>
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?<br>
</blockquote>
<div><br>
</div>
<div>No, the removal of the file extension was unintentional
(likely misunderstanding of my post-commit review
comment).</div>
<div> </div>
<blockquote class="gmail_quote" style="margin:0px 0px 0px
0.8ex;border-left:1px solid
rgb(204,204,204);padding-left:1ex">
<br>
When running the check from trunk i get the following
issues:<br>
<br>
$ clang-check google-readability-namespace-c<wbr>omments-cxx17.cpp
-- -std=c++17<br>
> error: invalid value 'c++17' in '-std=c++17'<br>
</blockquote>
<div><br>
</div>
<div>I suppose, your clang-check is just old. Clang supports
-std=c++17 these days.</div>
<div> </div>
<blockquote class="gmail_quote" style="margin:0px 0px 0px
0.8ex;border-left:1px solid
rgb(204,204,204);padding-left:1ex">
<br>
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.<br>
<br>
Running clang-tidy from command line does not produce
output (added fileextension for now):<br>
<br>
$ clang-tidy -checks=-*,google-readability-<wbr>namespace-comments
google-readability-namespace-c<wbr>omments-cxx17.cpp --
-std=c++1z<br>
$ clang-tidy -checks=-*,google-readability-<wbr>namespace-comments
google-readability-namespace-c<wbr>omments-cxx17.cpp -- --
-std=c++1z<br>
> Error while processing /home/jonas/opt/llvm/tools/cla<wbr>ng/tools/extra/test/clang-tidy<wbr>/google-readability-namespace-<wbr>comments-cxx17.cpp.<br>
<br>
Adding a 'std::cout << "Test if run" <<
std::endl' at the beginning of the `check` - Method works
and generates output:<br>
<br>
$ clang-tidy -checks=-*,google-readability-<wbr>namespace-comments
google-readability-namespace-c<wbr>omments-cxx17.cpp --
-std=c++1z<br>
Test if run<br>
Test if run<br>
Test if run<br>
<br>
So I think there is a regression in the Testingcode not
catching the nested namespaces. Did it work before and
what could have changed?<br>
If it did work and the clang-tidy code is the same this
could be a regression somewhere else.<br>
</blockquote>
<div><br>
</div>
<div>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).</div>
<div><br>
</div>
<div>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.</div>
<div> </div>
<blockquote class="gmail_quote" style="margin:0px 0px 0px
0.8ex;border-left:1px solid
rgb(204,204,204);padding-left:1ex">
<br>
All the Best, Jonas
<div class="gmail-HOEnZb">
<div class="gmail-h5"><br>
<br>
Am 11.10.2017 um 21:36 schrieb Aaron Ballman:<br>
<blockquote class="gmail_quote" style="margin:0px 0px
0px 0.8ex;border-left:1px solid
rgb(204,204,204);padding-left:1ex">
On Wed, Oct 11, 2017 at 3:29 PM, Alexander Kornienko
<<a href="mailto:alexfh@google.com"
target="_blank" moz-do-not-send="true">alexfh@google.com</a>>
wrote:<br>
<blockquote class="gmail_quote" style="margin:0px
0px 0px 0.8ex;border-left:1px solid
rgb(204,204,204);padding-left:1ex">
On Fri, Oct 6, 2017 at 3:27 PM, Aaron Ballman via
cfe-commits<br>
<<a href="mailto:cfe-commits@lists.llvm.org"
target="_blank" moz-do-not-send="true">cfe-commits@lists.llvm.org</a>>
wrote:<br>
<blockquote class="gmail_quote" style="margin:0px
0px 0px 0.8ex;border-left:1px solid
rgb(204,204,204);padding-left:1ex">
Author: aaronballman<br>
Date: Fri Oct 6 06:27:59 2017<br>
New Revision: 315060<br>
<br>
URL: <a
href="http://llvm.org/viewvc/llvm-project?rev=315060&view=rev"
rel="noreferrer" target="_blank"
moz-do-not-send="true">http://llvm.org/viewvc/llvm-pr<wbr>oject?rev=315060&view=rev</a><br>
Log:<br>
Renaming a test to start with the name of the
check based on post-commit<br>
review feedback; NFC.<br>
<br>
Added:<br>
<br>
clang-tools-extra/trunk/test/c<wbr>lang-tidy/google-readability-n<wbr>amespace-comments-cxx17<br>
</blockquote>
<br>
Sorry for not being clear. I didn't mean the
`.cpp` extension should be<br>
removed. This effectively disables the test, since
lit only runs tests in<br>
files with certain extensions (under
clang-tools-extra/test these are '.c',<br>
'.cpp', '.hpp', '.m', '.mm', '.cu', '.ll', '.cl',
'.s', '.modularize',<br>
'.module-map-checker', '.test').<br>
</blockquote>
That's entirely my fault -- I should have recognized
that. Sorry for<br>
the trouble!<br>
<br>
<blockquote class="gmail_quote" style="margin:0px
0px 0px 0.8ex;border-left:1px solid
rgb(204,204,204);padding-left:1ex">
I've just renamed the file to *.cpp and the test
fails for me:<br>
<br>
[0/1] Running the Clang extra tools' regression
tests<br>
FAIL: Clang Tools ::<br>
clang-tidy/google-readability-<wbr>namespace-comments-cxx17.cpp
(102 of 674)<br>
******************** TEST 'Clang Tools ::<br>
clang-tidy/google-readability-<wbr>namespace-comments-cxx17.cpp'
FAILED<br>
********************<br>
Script:<br>
--<br>
/usr/bin/python2.7<br>
/src/tools/clang/tools/extra/t<wbr>est/../test/clang-tidy/check_c<wbr>lang_tidy.py<br>
/src/tools/clang/tools/extra/t<wbr>est/clang-tidy/google-readabil<wbr>ity-namespace-comments-cxx17.<wbr>cpp<br>
google-readability-namespace-c<wbr>omments<br>
/build/tools/clang/tools/extra<wbr>/test/clang-tidy/Output/google<wbr>-readability-namespace-comment<wbr>s-cxx17.cpp.tmp<br>
-- -- -std=c++17<br>
--<br>
Exit Code: 1<br>
<br>
Command Output (stdout):<br>
--<br>
Running ['clang-tidy',<br>
'/build/tools/clang/tools/extr<wbr>a/test/clang-tidy/Output/googl<wbr>e-readability-namespace-commen<wbr>ts-cxx17.cpp.tmp.cpp',<br>
'-fix', '--checks=-*,google-readabilit<wbr>y-namespace-comments',
'--',<br>
'-std=c++17', '-nostdinc++']...<br>
------------------------ clang-tidy output
-----------------------<br>
<br>
------------------------------<wbr>------------------------------<wbr>------<br>
------------------------------ Fixes
-----------------------------<br>
<br>
------------------------------<wbr>------------------------------<wbr>------<br>
FileCheck failed:<br>
/src/tools/clang/tools/extra/t<wbr>est/clang-tidy/google-readabil<wbr>ity-namespace-comments-cxx17.<wbr>cpp:13:17:<br>
error: expected string not found in input<br>
// CHECK-FIXES: } // namespace n3<br>
^<br>
/build/tools/clang/tools/extra<wbr>/test/clang-tidy/Output/google<wbr>-readability-namespace-comment<wbr>s-cxx17.cpp.tmp.cpp:1:1:<br>
note: scanning from here<br>
// RUN: %check_clang_tidy %s
google-readability-namespace-c<wbr>omments %t --
--<br>
-std=c++17<br>
^<br>
/build/tools/clang/tools/extra<wbr>/test/clang-tidy/Output/google<wbr>-readability-namespace-comment<wbr>s-cxx17.cpp.tmp.cpp:5:7:<br>
note: possible intended match here<br>
// So that namespace is not empty.<br>
^<br>
<br>
<br>
--<br>
Command Output (stderr):<br>
--<br>
Traceback (most recent call last):<br>
File<br>
"/src/tools/clang/tools/extra/<wbr>test/../test/clang-tidy/check_<wbr>clang_tidy.py",<br>
line 140, in <module><br>
main()<br>
File<br>
"/src/tools/clang/tools/extra/<wbr>test/../test/clang-tidy/check_<wbr>clang_tidy.py",<br>
line 121, in main<br>
stderr=subprocess.STDOUT)<br>
File "/usr/lib/python2.7/subprocess<wbr>.py",
line 573, in check_output<br>
raise CalledProcessError(retcode, cmd,
output=output)<br>
subprocess.CalledProcessError: Command
'['FileCheck',<br>
'-input-file=/build/tools/clan<wbr>g/tools/extra/test/clang-tidy/<wbr>Output/google-readability-<wbr>namespace-comments-cxx17.cpp.<wbr>tmp.cpp',<br>
'/src/tools/clang/tools/extra/<wbr>test/clang-tidy/google-readabi<wbr>lity-namespace-comments-cxx17.<wbr>cpp',<br>
'-check-prefix=CHECK-FIXES',
'-strict-whitespace']' returned non-zero exit<br>
status 1<br>
<br>
--<br>
<br>
********************<br>
Testing Time: 13.07s<br>
********************<br>
Failing Tests (1):<br>
Clang Tools ::<br>
clang-tidy/google-readability-<wbr>namespace-comments-cxx17.cpp<br>
<br>
Expected Passes : 673<br>
Unexpected Failures: 1<br>
FAILED: tools/clang/tools/extra/test/C<wbr>MakeFiles/check-clang-tools<br>
<br>
<br>
Did you experience anything similar? Any ideas?<br>
</blockquote>
I now get the same behavior that you're seeing. I'm
not certain what's<br>
going on there (and don't have the time to look into
it at the<br>
moment), but perhaps Jonas has ideas.<br>
<br>
~Aaron<br>
<br>
<blockquote class="gmail_quote" style="margin:0px
0px 0px 0.8ex;border-left:1px solid
rgb(204,204,204);padding-left:1ex">
<blockquote class="gmail_quote" style="margin:0px
0px 0px 0.8ex;border-left:1px solid
rgb(204,204,204);padding-left:1ex">
- copied unchanged from r315059,<br>
clang-tools-extra/trunk/test/c<wbr>lang-tidy/google-readability-n<wbr>ested-namespace-comments.cpp<br>
Removed:<br>
<br>
clang-tools-extra/trunk/test/c<wbr>lang-tidy/google-readability-n<wbr>ested-namespace-comments.cpp<br>
<br>
Removed:<br>
clang-tools-extra/trunk/test/c<wbr>lang-tidy/google-readability-n<wbr>ested-namespace-comments.cpp<br>
URL:<br>
<a
href="http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clang-tidy/google-readability-nested-namespace-comments.cpp?rev=315059&view=auto"
rel="noreferrer" target="_blank"
moz-do-not-send="true">http://llvm.org/viewvc/llvm-pr<wbr>oject/clang-tools-extra/trunk/<wbr>test/clang-tidy/google-readabi<wbr>lity-nested-namespace-comments<wbr>.cpp?rev=315059&view=auto</a><br>
<br>
==============================<wbr>==============================<wbr>==================<br>
---<br>
clang-tools-extra/trunk/test/c<wbr>lang-tidy/google-readability-n<wbr>ested-namespace-comments.cpp<br>
(original)<br>
+++<br>
clang-tools-extra/trunk/test/c<wbr>lang-tidy/google-readability-n<wbr>ested-namespace-comments.cpp<br>
(removed)<br>
@@ -1,15 +0,0 @@<br>
-// RUN: %check_clang_tidy %s
google-readability-namespace-c<wbr>omments %t --<br>
-- -std=c++17<br>
-<br>
-namespace n1::n2 {<br>
-namespace n3 {<br>
- // So that namespace is not empty.<br>
- void f();<br>
-<br>
-// CHECK-MESSAGES: :[[@LINE+4]]:2: warning:
namespace 'n3' not terminated<br>
with<br>
-// CHECK-MESSAGES: :[[@LINE-7]]:11: note:
namespace 'n3' starts here<br>
-// CHECK-MESSAGES: :[[@LINE+2]]:3: warning:
namespace 'n1::n2' not<br>
terminated with a closing comment
[google-readability-namespace-<wbr>comments]<br>
-// CHECK-MESSAGES: :[[@LINE-10]]:11: note:
namespace 'n1::n2' starts here<br>
-}}<br>
-// CHECK-FIXES: } // namespace n3<br>
-// CHECK-FIXES: } // namespace n1::n2<br>
-<br>
<br>
<br>
______________________________<wbr>_________________<br>
cfe-commits mailing list<br>
<a href="mailto:cfe-commits@lists.llvm.org"
target="_blank" moz-do-not-send="true">cfe-commits@lists.llvm.org</a><br>
<a
href="http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits"
rel="noreferrer" target="_blank"
moz-do-not-send="true">http://lists.llvm.org/cgi-bin/<wbr>mailman/listinfo/cfe-commits</a><br>
</blockquote>
<br>
</blockquote>
</blockquote>
<br>
</div>
</div>
</blockquote>
</div>
<br>
</div>
</div>
</blockquote>
<br>
</body>
</html>