Le mar 24/02/2004 à 10:15, Eric Niebler a écrit :

Guillaume Melquiond wrote:

...

How did you deal with occurrences of min and max that rely on ADL (sorry if that is not the correct term)? Or did you handle only explicitly qualified calls std::min and std::max?

I'm asking because the Interval library relies on this kind of construct in order to deal with built-in types (like int, float, etc) and user-defined types (their min and max are found by Koenig lookup).

Hmmm ... I was aware of the theoretical possibility of this problem, but I didn't know there was actual boost code relying on ADL with std::min/std::max.

If there is a better way than Koenig lookup to allow the use of both std-defined and user-defined min and max (I'm only speaking about min and max here, but the library also has the same problem with abs, sin, sqrt, etc), I can change the library. Unfortunately I don't know of such a way (except requiring the user to define its own versions of the functions directly in the std namespace). Is the interval library the only Boost library allowing min and max functions to be used with user-defined types? How is this problem dealt with in the other libraries.

And the regression test with VC7.1 didn't expose this problem. (Shortcoming of vc7.1 or the interval regression tests?)

Yes it is a shortcoming of the interval regression tests (that doesn't mean it isn't also a shortcoming of VC7.1). I only test for things that could break in the current code, not for things somebody else could change afterward :-).

Getting the interval library to play nice with the min/max macros will take some work. Considering your code:

template< class T > void f(T const &a, T const &b) { using std::min; min(a,b); }

It *might* compile in the presence of the min() macro but it would likely do the wrong thing.

I don't remember exactly what the original problem was. Was it supposed to happen only with some compilers? Or with all the compilers that were using Microsoft headers?

It's unfortunate in this case that (min)(a,b) turns off ADL. (Out of curiosity, is there a case where it's not unfortunate? Ever since this language "feature" was brought to my attention last year, I have wondered about it.)

The obvious answer is to #undef min and max for the interval library, or at least #pragma push_macro/pop_macro for the compilers that support that. Yech.

Wasn't there the idea of adding prefix and suffix headers to Boost? If it was undef'd or pragma_push'd for all the libraries, it would avoid changing these 800 occurrences. Or am I missing something?

Another thought just occured to me. Consider:

#define BOOST_EMPTY template< class T > void f(T const &a, T const &b) { using std::min; min BOOST_EMPTY (a,b); }

This seems to be enough to prevent min() macro substitution, and it would preserve ADL. I checked with VC6, VC7.1, gcc (cygwin) and with Comeau Online, and they all like it. Will this work on other compilers too?

It's ugly. But I suppose we could use a boost_min macro to avoid cluttering the code (if it still works). Regards, Guillaume

Le mar 24/02/2004 à 18:31, Eric Niebler a écrit :

Guillaume Melquiond wrote:

[...]

...

Yes it is a shortcoming of the interval regression tests (that doesn't mean it isn't also a shortcoming of VC7.1). I only test for things that could break in the current code, not for things somebody else could change afterward :-).

IMO, I think you should add a test for this. It's a valid usage scenario which you support. You (and I, as a maintainer) want to know when it breaks.

Will do.

...

Wasn't there the idea of adding prefix and suffix headers to Boost? If it was undef'd or pragma_push'd for all the libraries, it would avoid changing these 800 occurrences. Or am I missing something?

Not all compilers support push_macro/pop_macro, and unconditionally removing users' macros is impolite, considering there are ways we can make boost code impervious to these macros.

No, it's not what I meant. I wasn't suggesting undef'ing once and for all these macros. Since there is a suffix header, they could be restored. Or is it impossible to do with basic preprocessing? [...]

I agree it's ugly. But not as ugly as the current solution in win32.hpp, which can silently change the meaning of users' code. You don't need a boost_min macro -- you can define your own interval_min/interval_max templates as follows:

#define BOOST_EMPTY template< class T > inline void interval_min(T const &a, T const &b) { using std::min; min BOOST_EMPTY (a,b); }

and now use interval_min instead of min everywhere. Perhaps we can do this once and call it boost_min, so everybody can use it. Opinions?

Seems fine. In fact, it would be a bit more complex since min and max are not always in the std namespace, they can also be in the root namespace (even if they should not). The Interval library tests which compiler and standard library are used in order to find the correct namespace. But it's only a minor detail. Regards, Guillaume

Eric Niebler wrote:

Was that a vote in favor of an interval_min function, or a boost-wide utility? If we want a boost-wide utility (a good thing to have, IMO), what should it be called? boost::boost_min (kind of repetitive)? or perhaps boost::dependent_min to emphasize the fact that is uses Koenig lookup? Or just boost::min_?

Why not having both std::min and "dependent" min: #define BOOST_EMPTY template< class T > inline T const & const std_min(T const &a, T const &b) { return std::min BOOST_EMPTY (a, b); } template< class T > inline T const & dependent_min(T const &a, T const &b) { using std::min; return min BOOST_EMPTY (a, b); } possibly adding suitable workarounds for implementation that put min in the global namespace. I like the idea of having those as boost-wide utilities. I think that std_min(x, y) is nicer and more descriptive to read than (min)(x, y), but that's my personal taste. The separate name dependent_min make it self-evident even to the causal reader that it's something different from std::min/std_min. Just my €0.01 Alberto Barbati

"Eric Niebler" <eric@boost-consulting.com> writes:

Guillaume Melquiond wrote:

...

How did you deal with occurrences of min and max that rely on ADL (sorry if that is not the correct term)? Or did you handle only explicitly qualified calls std::min and std::max? I'm asking because the Interval library relies on this kind of construct in order to deal with built-in types (like int, float, etc) and user-defined types (their min and max are found by Koenig lookup).

Hmmm ... I was aware of the theoretical possibility of this problem, but I didn't know there was actual boost code relying on ADL with std::min/std::max.

I don't see the problem. Wasn't the intention only to replace qualified std::min and std::max with (std::min) and (std::max)? -- Dave Abrahams Boost Consulting www.boost-consulting.com

-----Original Message----- [mailto:boost-bounces@lists.boost.org] On Behalf Of Eric Niebler

David Abrahams wrote:

...

"Eric Niebler" <eric@boost-consulting.com> writes:

...

...

Hmmm ... I was aware of the theoretical possibility of this problem, but I didn't know there was actual boost code relying on ADL with std::min/std::max.

I don't see the problem. Wasn't the intention only to replace qualified std::min and std::max with (std::min) and (std::max)?

The intention was to remove the min/max hack from win32.hpp, and then fix all the places where the min/max macros would otherwise wreak havoc. That isn't limitted to qualified called to std::min and std::max. It also includes unqualified calls to min/max, and also calls to numeric_limits<foo>::max, and the declarations and invocations of all min/max member functions.

Provided that usages of min/max aren't used as arguments to macros, you can solve an evil preprocessor problem like this with an annoying but effective preprocessor solution: #define DO_NOT_EXPAND std::min DO_NOT_EXPAND(a, b) std::max DO_NOT_EXPAND(a, b) I.e. no ADL problems and no macro expansion. However, if this is used as a macro *argument*, secondary scannings of the token sequence will still cause min/max to expand. #define ID(x) x ID(std::min DO_NOT_EXPAND(a, b)) // will (and correctly should) expand Further, VC++ has an annoying bug that will cause it to expand anyway if it appears in the replacement list of another macro: #define less(a, b) ((a) < (b)) // e.g. #define ABC less DO_NOT_EXPAND(x, y) less DO_NOT_EXPAND(x, y) // right, expands to: less (x, y) ABC // wrong, expands to: ((x) < (y)) Regards, Paul Mensonides

Daryle Walker wrote:

This "solution" looks extremely invasive, given all the different places that could need changing. Maybe it's not worth it, considering the hit that anyone not using the Windows headers (automatically including anyone not using Windows nor an IBM-styled PC) takes. What was wrong with limiting the workaround to "win32.hpp"?

You're a little late -- the fix is done and checked in. No end users take a "hit" as a result of this change. There were several things wrong with limiting the workaround to win32.hpp and the have been discussed extensively on this list. Please see the archive. Thanks. -- Eric Niebler Boost Consulting www.boost-consulting.com

David Abrahams wrote:

They all compile and run the following:

namespace std { template<class T> const T& min(const T& a, const T& b) { return a<b?a:b; } } int main() { int x = 0; int y = 1; return (std::min)(x,y); }

But where will the definition of min come from on broken stdlib implementations?

Changing a call from min(a,b) to (min)(a,b) would break on stdlib implementations that *only* have the min/max macros and not the min/max algorithms, is that what you're saying? Do such implementations exist? We'll have to provide the min/max algorithms for such platforms, I suppose. Which platforms are affected?

...

Also, I'm not a CVS guru. If all hell breaks loose as a result of this change, is there someone willing to help me back it out?

Just "cvs tag" to mark the state before you start making changes and it'll be comparitively easy.

Guess I should have done that before the 800 edits, huh? Hmm ...

...

4) This is the sort of thing developers will forget to do even if it's in a style guide. A simple way of ensuring this stays fixed would be to run the regressions with min() and max() #defined to garbage, so that violations are detected and corrected early. Is this possible?

Yes, it's possible, but that'll only detect the usage:

min(x,y)

and not

std::min(x,y)

both of which are going to be banned, right?

Why would it not detect the second? If I #define min(a,b) to garbage, then the above becomes: std::garbage; Mission accomplished. -- Eric Niebler Boost Consulting www.boost-consulting.com

I just committed the min/max macro fixes. I built and tested with vc6, vc7.1 and gcc (cygwin). This was a huge change and I have little doubt that it will cause problems somewhere. Let me know if you see anything out of the ordinary. This could potentially have downstream effects, particularly if people are relying on the boost headers to #undef the min and max macros for them, which they no longer do. With this check-in, I did *not* modify the regression tests to #define min and max to garbage. That caused too many problems. In the future, calling min and max (member) functions will take some care on the part of all boost developers. I'd like to write up a document about what I learned. Is there a boost style guide, or an appropriate place for such a document? Thanks. <runs for cover> -- Eric Niebler Boost Consulting www.boost-consulting.com

Edward Diener wrote:

Yes, you definitely need to document this if the normal calls of std::min(x,y) or std::max(x,y) no longer work properly when including Boost header files for normal developers also. I do know of the workaround for VC++ of (std::min)(x,y) and (std::max)(x,y) which I have to use in my own code when windows.h is included. If things have gotten more complicated than this when Boost headers are included, programmers must definitely be told about or else one is going to have many unhappy developers on one's hands. I would have said to you that you need to write documentation on the changes before committing the fixes, and not considering the importance of doing so as an aftermath, even if only Boost developers are involved, but since I am not a Boost developer myself (yet) I will shut up further about it. <g>

I know documentation is needed. I've asked twice if there is an appropriate place for such documentation. I'm still waiting. Also, I think you misunderstand the problem and the fix. The boost headers do not *introduce* any new problems with min/max. Any code that compiles without the boost headers should continue to compile fine with them. And I haven't sprung this change on anyone. I've discussed it on this list for quite some time, and asked time and again about procedure. If you had a problem, you should have spoken up. I am happy to write the documentation. Just tell me where to put it. -- Eric Niebler Boost Consulting www.boost-consulting.com

This concerns all boost developers. Please read! I have just added documentation for how to deal with min/max in boost code. I have added it to the "Boost Library Requirements and Guidelines" document (boost/more/lib_guide.htm). That seemed the appropriate place, but it might be hard to find. If anyone thinks this belongs in a more prominent place, please suggest one. For reference, here is the text I have added: ---- Make sure your code compiles in the presence of the min() and max() macros. Some platform headers define min() and max() macros which cause some common C++ constructs to fail to compile. Some simple tricks can protect your code from inappropriate macro substitution: * If you want to call std::min() or std::max(): o Use (std::min)(a,b) if you do not require argument-dependent look-up. o Use boost::std_min(a,b) if you do require argument-dependent look-up. boost::std_min() delegates to std::min(). * If you want to call std::numeric_limits<int>::max(), use (std::numeric_limits<int>::max)() instead. * If you want to call a min() or max() member function, instead to doing obj.min(), use (obj.min)(). * If you want to declare or define a function or a member function named min or max, then you must use the BOOST_PREVENT_MACRO_SUBSTITUTION macro. Instead of writing int min() { return 0; } you should write int min BOOST_PREVENT_MACRO_SUBSTITUTION () { return 0; } This is true regardless if the function is a free (namespace scope) function, a member function or a static member function, and it applies for the function declaration as well as the function definition. -- Eric Niebler Boost Consulting www.boost-consulting.com

Eric, Eric Niebler wrote:

This concerns all boost developers. Please read!

* If you want to call std::min() or std::max():

o Use (std::min)(a,b) if you do not require argument-dependent look-up. o Use boost::std_min(a,b) if you do require argument-dependent look-up. boost::std_min() delegates to std::min().

I do not get it. IIUC ADL does not apply to a qualified name. I.e. std::min is just std::min and that's it. ?? Thomas

Paul Mensonides wrote:

...

-----Original Message----- From: boost-bounces@lists.boost.org [mailto:boost-bounces@lists.boost.org] On Behalf Of Thomas Witt

...

I do not get it. IIUC ADL does not apply to a qualified name. I.e. std::min is just std::min and that's it.

You have to do it to prevent macro expansion from a definition like this:

#define min(x, y) ((x) < (y) ? (x) : (y))

std::min(1, 2) // std::((1) < (2) ? (1) : (2)) (std::min)(1, 2) // (std::min)(1, 2)

Regards, Paul Mensonides

Thanks Paul. But I do understand the reason why we need to disable macro expansion (at least I think so). I was referring to the following sentence o Use boost::std_min(a,b) if you do require argument-dependent look-up. boost::std_min() delegates to std::min(). I can't see where ADL comes into play here. Thomas

Paul Mensonides wrote:

The only way to "have-your-cake-and eat-it-too" in the presence of macros like min/max is to prevent them from expanding manually with BOOST_PREVENT_MACRO_SUBSTITUTION.

int main() { using std::min; min BOOST_PREVENT_MACRO_SUBSTITUTION(1, 2); return 0; }

This yields ADL and local context (which includes std::min if necessary).

Thanks, Paul. That's correct. boost::std_min is defined as follows: template< typename T > inline T const & std_min( T const & a, T const & b ) { using std::min; return min BOOST_PREVENT_MACRO_SUBSTITUTION ( a, b ); } So a call to boost::std_min is essentially an unqualified call to min, with a using directive to pull in std::min. Since it's an unqualified call, ADL can also pull in additional overloads from different namespaces. boost::std_min is nothing more than a convenience to keep you from littering your code with using statements and Ugly Macros. I might hazard to make a generalization here and suggest you prefer std_min to std::min because (a) chances are good you want ADL anyway, (b) std_min doesn't suffer from macro mish-mash, and (c) std_min is 1 fewer character to type when you're already in the boost namespace. :-) Hope that makes sense now. -- Eric Niebler Boost Consulting www.boost-consulting.com

Daniel Frey <d.frey@gmx.de> writes:

Eric Niebler wrote:

...

boost::std_min is defined as follows: template< typename T > inline T const & std_min( T const & a, T const & b ) { using std::min; return min BOOST_PREVENT_MACRO_SUBSTITUTION ( a, b ); }

I don't like the name of the function, as std_min doesn't hint me about the ADL-nature of the call. Would adl_std_min be too much typing? Or propbably just adl_min, as it's quite common to use std::min if ADL doesn't find a better match. My $.02.

Looks to me as though boost::min_ is the best name for this thing. -- Dave Abrahams Boost Consulting www.boost-consulting.com

On Sun, 29 Feb 2004 09:31:40 -0800, Eric Niebler wrote

...

Daniel Frey <d.frey@gmx.de> writes:

...

I don't like the name of the function, as std_min doesn't hint me about the ADL-nature of the call. Would adl_std_min be too much typing? Or propbably just adl_min, as it's quite common to use std::min if ADL doesn't find a better match. My $.02.

Looks to me as though boost::min_ is the best name for this thing.

I tried to engage people in a discussion of these issues before making 1000+ edit to correct this problem. Now, everybody want to change it. :-P

This would be a simple search/replace, so I don't mind. Can we all agree on boost::min_? I don't want to have to change it again.

No, I don't think it is worth changing again. min_ isn't better than std_min in my view. Put extra comments in the code / docs to explain it. Changing it again means all of us will have to re-integrate your new changes. It's working, don't mess with it, lets move on. Jeff ps: Thx for taking this on. I haven't personally had to deal with this nasty bit of windows macro hell, but I'm sure it would have got me sooner or later.

Eric Niebler wrote:

Paul Mensonides wrote:

...

The only way to "have-your-cake-and eat-it-too" in the presence of macros like min/max is to prevent them from expanding manually with BOOST_PREVENT_MACRO_SUBSTITUTION.

int main() { using std::min; min BOOST_PREVENT_MACRO_SUBSTITUTION(1, 2); return 0; }

This yields ADL and local context (which includes std::min if necessary).

Thanks, Paul. That's correct. boost::std_min is defined as follows:

template< typename T > inline T const & std_min( T const & a, T const & b ) { using std::min; return min BOOST_PREVENT_MACRO_SUBSTITUTION ( a, b ); }

So a call to boost::std_min is essentially an unqualified call to min, with a using directive to pull in std::min. Since it's an unqualified call, ADL can also pull in additional overloads from different namespaces.

Maybe it's worth mentioning that it is only usefull for use in the namespace boost. IIUC there is no lookup in the local namespace and additional lookup in boost. I.e. it's not quite an unqualified call when used outside boost. Thomas

On 2/28/04 5:26 PM, "Eric Niebler" <eric@boost-consulting.com> wrote:

This concerns all boost developers. Please read!

I have just added documentation for how to deal with min/max in boost code. I have added it to the "Boost Library Requirements and Guidelines" document (boost/more/lib_guide.htm). That seemed the appropriate place, but it might be hard to find. If anyone thinks this belongs in a more prominent place, please suggest one.

For reference, here is the text I have added: [TRUNCATE]

In another post, I mentioned an alternate fix to this problem. Here, I'll talk about your specific fix. You seem to be assuming: The Windows header macros are Boost-hostile But from recent posts about the problem, I think: The Windows header macros are Boost-hostile _and_ STL-hostile We can "fix" all the Boost code, but we can't change the STL files. Since Boost (pretty much) requires STL, the user is screwed anyway. This means that there was no point in changing our code! I've heard that the Windows headers themselves don't use/need the macros, they would only exist in user code. If the macros are disabled, then the user can change to the STL versions. If we force the user to disable the macros, then the user can do an one-time change to his/her code to be Boost/STL compliant _and_ Windows compliant (since Windows doesn't break if the macros are disabled). The current "fix" forces all Boost developers to rewrite code forevermore because of some obsolete code, even if the developer doesn't use Windows! I consider the current state to be a _horrible_ trade-off. If you're burnt-out from adding the "fix", I'll volunteer to rip it all out. Just give me a list of all the files you changed. (If it can be undone from CVS, without screwing up any changes that happened afterwards, then you can do that instead.) -- Daryle Walker Mac, Internet, and Video Game Junkie darylew AT hotmail DOT com

"Edward Diener" <eddielee@tropicsoft.com> wrote in message news:c1m13g$tro$1@sea.gmane.org...

header files for normal developers also. I do know of the workaround for VC++ of (std::min)(x,y) and (std::max)(x,y) which I have to use in my own code when windows.h is included.

Do you refer to library source code that you distribute to multiple customers or to internal/application code? If the latter, why not use -DNOMINMAX? I can understand the issue with Boost sources, but otherwise adding NOMINMAX to the list of pre-defined symbols becomes pretty much reflexive when developing under Windows... I don't remember encountering any SDK or other Microsoft header files that rely on min/max macros and only ran across a 3rd party header that expected them once. ...Max...