Boost.Test: violation of min/max guidelines
I've just been hit by Boost.Test not following the min/max coding guidelines, the patch is trivial: Index: boost/test/floating_point_comparison.hpp =================================================================== RCS file: /cvsroot/boost/boost/boost/test/floating_point_comparison.hpp,v retrieving revision 1.26 diff -r1.26 floating_point_comparison.hpp 66c66 < return std::numeric_limits<FPT>::max(); ---
return (std::numeric_limits<FPT>::max)();
Needs to be applied to the release branch as well now of course :-) Thanks, John.
"John Maddock" <john@johnmaddock.co.uk> wrote in message news:01bf01c64d19$14cdf180$32311b56@fuji...
I've just been hit by Boost.Test not following the min/max coding guidelines, the patch is trivial:
Index: boost/test/floating_point_comparison.hpp =================================================================== RCS file: /cvsroot/boost/boost/boost/test/floating_point_comparison.hpp,v retrieving revision 1.26 diff -r1.26 floating_point_comparison.hpp 66c66 < return std::numeric_limits<FPT>::max(); ---
return (std::numeric_limits<FPT>::max)();
Needs to be applied to the release branch as well now of course :-)
Thanks, John.
I though Volodya fixed this along with other max. Anyway, could you commit this? Gennadiy
John Maddock wrote:
I've just been hit by Boost.Test not following the min/max coding guidelines, the patch is trivial:
Index: boost/test/floating_point_comparison.hpp =================================================================== RCS file: /cvsroot/boost/boost/boost/test/floating_point_comparison.hpp,v retrieving revision 1.26 diff -r1.26 floating_point_comparison.hpp 66c66 < return std::numeric_limits<FPT>::max(); ---
return (std::numeric_limits<FPT>::max)();
Can I ask what is the purpose of adding parenthesis to the qualified name? ADL should not take place for the qualified name? Do some compilers get this wrong? Is there another reason? Regards, Richard
Richard Corden wrote: [...]
Can I ask what is the purpose of adding parenthesis to the qualified name?
ADL should not take place for the qualified name? Do some compilers get this wrong?
Is there another reason?
See http://www.boost.org/more/lib_guide.htm#Guidelines. Markus
"Markus Schöpflin" wrote
Richard Corden wrote:
[...]
Can I ask what is the purpose of adding parenthesis to the qualified name?
ADL should not take place for the qualified name? Do some compilers get this wrong?
Is there another reason?
FYI I have discovered that in my useage of min the advice doesnt seem to help, my use being default construction of a temporary of type named max or min:
From one of my test files I currently ended up using the obvious workaround:
// here boost::pqs::time::min is a typedef for a quantity of time in minutes #if ! defined (min) BOOST_CHECK(units_str( boost::pqs::time::min()) == "min"); #else //use of min() looks like the macro so just create a variable boost::pqs::time::min min; BOOST_CHECK(units_str(min) == "min"); #endif The only other option would seem to be to "save" the body of the min macro undefine it and then redefine it later, IOW not possible in practise AFAIK. If anyone has ideas what the preferred option is in this situation I would be interested to know? ---------- Boost.Inspect and min max Inspect reports the above as a violation of boost guidelines of course, though I have attended to them. I dont know quite what else to do about it as the names are the recommended SI names. Ideally I would like to tell inspect to disable this "violation" here as I've attended to it etc. Inspect also reports use of min in comments as in the 2nd /3rd line below as a violation: #if defined (min) // some compilers use a min() macro // this causes a failure of useage of e.g my_stuff::min() // for other purposes #define DONT_USE_MIN_LIKE_CONSTRUCT #endif Finally Boost.Inspect reports use in strings as a violation: std::string my_min_str = "just a min()"; I will try to come up with a patch for Boost.Inspect. ... Meanwhile FYI. regards Andy Little
FYI I have discovered that in my useage of min the advice doesnt seem to help, my use being default construction of a temporary of type named max or min:
From one of my test files I currently ended up using the obvious workaround:
How about: BOOST_CHECK(units_str( boost::pqs::time::min BOOST_PREVENT_MACRO_SUBSTITUTION()) == "min"); John.
"John Maddock" <john@johnmaddock.co.uk> wrote in message news:018f01c64db1$03cca050$65330252@fuji...
FYI I have discovered that in my useage of min the advice doesnt seem to help, my use being default construction of a temporary of type named max or min:
From one of my test files I currently ended up using the obvious workaround:
How about:
BOOST_CHECK(units_str( boost::pqs::time::min BOOST_PREVENT_MACRO_SUBSTITUTION()) == "min");
using VC7.1 and I enabling language extensions ( reqd for e.g Windows.h) that comes up at the above line (N) with: sourcefile(N) :warning C4003: not enough actual parameters for macro 'min' sourcefile(N) :warning C4003: not enough actual parameters for macro 'min' sourcefile(N) :error C2589: '(' : illegal token on right side of '::' sourcefile(N) :error C2143: syntax error : missing ')' before '::' sourcefile(N) :error C2059: syntax error : ')' sourcefile(N) :error C3861: 'units_str': identifier not found, even with argument-dependent lookup regards Andy Little
Andy Little wrote:
FYI I have discovered that in my useage of min the advice doesnt seem to help, my use being default construction of a temporary of type named max or min:
From one of my test files I currently ended up using the obvious workaround:
// here boost::pqs::time::min is a typedef for a quantity of time in minutes #if ! defined (min) BOOST_CHECK(units_str( boost::pqs::time::min()) == "min"); #else //use of min() looks like the macro so just create a variable boost::pqs::time::min min; BOOST_CHECK(units_str(min) == "min"); #endif
The only other option would seem to be to "save" the body of the min macro undefine it and then redefine it later, IOW not possible in practise AFAIK.
If anyone has ideas what the preferred option is in this situation I would be interested to know?
See John's reply. Another option would be: typedef boost::pqs::time::min min_; BOOST_CHECK(units_str( min_()) == "min");
---------- Boost.Inspect and min max
Inspect reports the above as a violation of boost guidelines of course, though I have attended to them. I dont know quite what else to do about it as the names are the recommended SI names. Ideally I would like to tell inspect to disable this "violation" here as I've attended to it etc.
Inspect also reports use of min in comments as in the 2nd /3rd line below as a violation:
#if defined (min) // some compilers use a min() macro // this causes a failure of useage of e.g my_stuff::min() // for other purposes #define DONT_USE_MIN_LIKE_CONSTRUCT #endif
Finally Boost.Inspect reports use in strings as a violation:
std::string my_min_str = "just a min()";
I will try to come up with a patch for Boost.Inspect.
A patch would be welcome. Ideally, Inspect should use Wave and ignore violations in comments and string literals. Thanks! -- Eric Niebler Boost Consulting www.boost-consulting.com
"Eric Niebler" wrote
A patch would be welcome. Ideally, Inspect should use Wave and ignore violations in comments and string literals.
Though preprocessing might complicate matters for this particular problem surely ;-) . Unless you mean modify wave to find use of min and max? regards Andy Little
Markus Schöpflin wrote:
Richard Corden wrote:
[...]
Can I ask what is the purpose of adding parenthesis to the qualified name?
ADL should not take place for the qualified name? Do some compilers get this wrong?
Is there another reason?
The guidelines actually confused me a little. The first bullet for calling min or max has: If you want to call std::min() or std::max(): * If you do not require argument-dependent look-up, use (std::min)(a,b). I got caught up in the ADL part and didn't realise that the parenthesis were there to stop macro expansion not to stop ADL. Regards, Richard
Richard Corden wrote: [...]
return (std::numeric_limits<FPT>::max)();
Can I ask what is the purpose of adding parenthesis to the qualified name?
It inhibits macro expansion.
ADL should not take place for the qualified name? Do some compilers get this wrong?
Some environments still insist on #defining macros for min and max. Regards, m Send instant messages to your online friends http://au.messenger.yahoo.com
Martin Wille wrote:
Richard Corden wrote:
[...]
return (std::numeric_limits<FPT>::max)();
Can I ask what is the purpose of adding parenthesis to the qualified name?
It inhibits macro expansion.
I'm blown away. It never occurred to me that macro expansion would happen for a qualified name, but of course now it seems just, well, obvious. Thanks, Richard
Richard Corden <richard.corden@gmail.com> writes:
Can I ask what is the purpose of adding parenthesis to the qualified name?
ADL should not take place for the qualified name? Do some compilers get this wrong?
Is there another reason?
The reason is #define max(a,b) ((a)>(b)?(a):(b)) :) -- Dave Abrahams Boost Consulting www.boost-consulting.com
participants (8)
-
Andy Little -
David Abrahams -
Eric Niebler -
Gennadiy Rozental -
John Maddock -
Markus Schöpflin -
Martin Wille -
Richard Corden