<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">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">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">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">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">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">cfe-commits@lists.llvm.org</a><br>
<a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits" rel="noreferrer" target="_blank">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>