[cfe-commits] r163429 - /cfe/trunk/tools/clang-check/ClangCheck.cpp

[Douglas Gregor]

Sent from my iPhone

On Sep 8, 2012, at 6:55 PM, Chandler Carruth <chandlerc at google.com> wrote:

> On Sat, Sep 8, 2012 at 6:10 PM, Douglas Gregor <dgregor at apple.com> wrote:
>> 
>> 
>> Sent from my iPhone
>> 
>> On Sep 7, 2012, at 6:44 PM, Alexander Kornienko <alexfh at google.com> wrote:
>> 
>> > Author: alexfh
>> > Date: Fri Sep  7 17:44:34 2012
>> > New Revision: 163429
>> >
>> > URL: http://llvm.org/viewvc/llvm-project?rev=163429&view=rev
>> > Log:
>> > Fixed http://llvm.org/bugs/show_bug.cgi?id=13777
>> >
>> > Modified:
>> >    cfe/trunk/tools/clang-check/ClangCheck.cpp
>> >
>> > Modified: cfe/trunk/tools/clang-check/ClangCheck.cpp
>> > URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/tools/clang-check/ClangCheck.cpp?rev=163429&r1=163428&r2=163429&view=diff
>> > ==============================================================================
>> > --- cfe/trunk/tools/clang-check/ClangCheck.cpp (original)
>> > +++ cfe/trunk/tools/clang-check/ClangCheck.cpp Fri Sep  7 17:44:34 2012
>> > @@ -58,7 +58,9 @@
>> >     "ast-dump-filter",
>> >     cl::desc(Options->getOptionHelpText(options::OPT_ast_dump_filter)));
>> >
>> > -namespace {
>> > +// Anonymous namespace here causes problems with gcc <= 4.4 on MacOS:
>> > +// http://llvm.org/bugs/show_bug.cgi?id=13777
>> > +// namespace {
>> 
>> How about just putting it in the clang namespace? At the very least, please delete the commented-out lines.
> 
> Agreed on both fronts. Also, bugs are usually cited just as "PR13777", and almost never cited in the source code, but rather in the test case itself.

The reasoning behind this is that the code and comments should stand alone. Nobody should have to hunt through a bug tracker to determine the intent. 

The test suite, on the other hand, is a mix of feature tests and regression tests that often grows in response to specific bugs, and the cross-linking to those bugs is a valuable addition. 

> That said, is this really the necessary solution? There are a lot of template arguments defined in an anonymous namespace within the codebase already. How do they work without problems?

Yes, this is odd. 

> I feel like this is an ODR violation waiting to happen unless we put it in an anonymous namespace the way it should be...
> 
> -Chandler
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20120909/c331c350/attachment.html>