berkus / include-what-you-use

Automatically exported from code.google.com/p/include-what-you-use
Other
1 stars 0 forks source link

Invalid code in test #101

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago
The fwd_decl_nested_class test contained invalid code. Redeclarations of 
members are apparently illegal, as noted in C++ standard defect #85 [1].

DR85 was implemented in Clang r183722, which made this test throw a GNU 
extension warning.

After discussion on the Clang list [2], I came to the conclusion that this is 
invalid code, and IWYU doesn't necessarily need to support it.

This patch removes the redeclaration in our test case.

Thanks for any comments on policy decision, not much content to review.

[1] http://www.open-std.org/jtc1/sc22/wg21/docs/cwg_defects.html, search for 
"Redeclaration of member class"
[2] http://comments.gmane.org/gmane.comp.compilers.clang.scm/74782

Original issue reported on code.google.com by kim.gras...@gmail.com on 6 Jul 2013 at 7:39

Attachments:

GoogleCodeExporter commented 9 years ago
OK, agree. No need to test invalid code. But I don't think it's necessary to 
modify IWYU code which tells that NoUsageDefinedOutOfLine should be removed.

Original comment by vsap...@gmail.com on 7 Jul 2013 at 7:45

GoogleCodeExporter commented 9 years ago
Originally, I hadn't thought to do so, but I stumbled over the relevant section 
in iwyu.cc:
https://code.google.com/p/include-what-you-use/source/browse/trunk/iwyu.cc#3419

It seems to me this search for the first decl is pointless, as the language 
only allows for one. And if someone is using the language extension to allow 
nested class redeclarations, there's no real harm to leaving them all.

But I don't completely understand the effects of removing this check, so I'll 
leave it in for now. It seems harmless.

Thanks for review!

Original comment by kim.gras...@gmail.com on 10 Jul 2013 at 8:36

GoogleCodeExporter commented 9 years ago
Fixed in r476

Original comment by kim.gras...@gmail.com on 10 Jul 2013 at 8:40