Opened 12 years ago

Closed 12 years ago

Last modified 12 years ago

#4527 closed Bugs (fixed)

warning in Boost.format and Boost.test C4224

Reported by: Paul A. Bristow Owned by: Samuel Krempp
Milestone: Boost 1.44.0 Component: format
Version: Boost Development Trunk Severity: Cosmetic
Keywords: format C4224 Cc:

Description

Creates a blizzard of C2442 warnings when called from Boost.Test.

I:\boost_1_43_0\boost/format/alt_sstream.hpp(128): warning C4224: nonstandard extension used : formal parameter 'arg' was previously defined as a type

I:\boost_1_43_0\boost/format/alt_sstream.hpp(145) : see reference to class template instantiation 'boost::io::basic_oaltstringstream<Ch,Tr,Alloc>::No_Op' being compiled with [

Ch=char, Tr=std::char_traits<char>, Alloc=std::allocator<char>

]

Problem is in class basic_oaltstringstream

template<class T>

const T & operator()(const T & arg) { return arg; }

This trivial change of name from "arg" to "arg1" cures it.

template<class T> const T & operator()(const T & arg1) { return arg1; }

Change History (6)

comment:1 by Steven Watanabe, 12 years ago

Here's a smaller test case:

#include <boost/bind.hpp>
#include <boost/format.hpp>

This produces warnings with /Za /W4 on MSVC 2008 and 2010.

The problem reduces to

namespace boost {

template<int I>
class arg {};

namespace io {

class test {
    int operator()(const int & arg) { return arg; }
};

}
}

Frankly, this is just broken. I suggest that you just disable the warning or don't use /Za.

comment:2 by Paul A. Bristow, 12 years ago

Keywords: C4224 added; C2442 removed

Well to the hapless Boost.Math library user who just wants a quick Student's t value and gets a faceful of warnings in a library (Boost.Format) he didn't even know he was using, it is certainly broken!

I'm not a language lawyer or C++ style guru, but to reuse a name a bit gratuitously doesn't feel the best programming style ?

It is even simpler than your test case

class arg {};
 
 class test
 {
     int operator()(const int & arg)
     {
       return arg; // Warning 4224 - argument arg same name as class arg.
     }
 };

 class test_OK
 {
     int operator()(const int & arg1)
     {
       return arg1; // OK argument has different name from class.
     }
 };
/*
test_naming.cpp(5) : warning C4224: nonstandard extension used
: formal parameter 'arg' was previously defined as a type

Microsoft (R) Windows (R) Resource Compiler Version 6.0.5724.0
*/

I'm not clear what is the objection to the pragmatic simple change of name from 'arg' to anything else.

Won't it get the problem out of our (and users) hair until MS (claiming it is an extension) and WG21 reach agreement if it is 'legal' or not?

comment:3 by Steven Watanabe, 12 years ago

The code is perfectly fine. To see just how much code would have to be changed to avoid this warning entirely try running:

grep -R -I --exclude-dir=.svn --exclude-dir=bind --exclude-dir=mpl '\barg\b' boost

Reusing a name in this context is not a stylistic problem at all. The use of arg in Boost.Bind is totally irrelevant to Boost.Format. When I'm writing an argument name in my own member function in my own class in my own namespace, I don't want to have to worry about any other part of Boost. In order to prevent it we can't use any class name in any parent namespace as an argument name. This means that classes added in the global namespace (which we have no control over whatsoever) can trigger this warning in random parts of Boost.

Also, /Za is not turned on by default anyway.

comment:4 by Paul A. Bristow, 12 years ago

OK - instead of my sticking plaster fix, I have

1 'Complained' to Microsoft that this code is well-formed and should not trigger this warning, even with (no extensions) /Za option on. Long term this should cure this persistent problem.

2 Meanwhile, I have suppressed this warning in examples and tests that I am writing.

Also, /Za is not turned on by default anyway.

But users who try (even if perhaps ineffectively) to attain strict standard compliance, should not be inconvenienced by a massive load of warning messages, or worse by a failure to compile if their policy is to treat warnings as errors.

comment:5 by Paul A. Bristow, 12 years ago

Resolution: fixed
Status: newclosed

in reply to:  5 comment:6 by Paul A. Bristow, 12 years ago

Replying to pbristow:

Microsoft Response

Thank you for filing this issue. You are correct in that this is a bug in our compiler, however, there are some mitigating factors here. This warning is off-by-default in the compiler, and can only be turned on with /Wall (which is rare), or using /Za. And, as with all warnings, it is not an error unless combined with /WX (even under /Za) and can always be disabled with the compiler switch /wd or #pragma warning. Given these mitigating factors, we have prioritized this bug below more severe issues, and are not planning to fix this in our next release.

Thank you for bringing this issue to our attention!

Andy Rich Visual C++ QA

http://connect.microsoft.com/VisualStudio/feedback/details/586147/warning-c4224-is-given-for-correct-code-if-ms-extensions-are-disabled

I've replied 'thanks' ;-)

Note: See TracTickets for help on using tickets.