Bug 49386 - #undef min/max is dependent on order if #include
Summary: #undef min/max is dependent on order if #include
Status: RESOLVED WONTFIX
Alias: None
Product: gcc
Classification: Unclassified
Component: libstdc++ (show other bugs)
Version: 4.7.0
: P3 normal
Target Milestone: ---
Assignee: Not yet assigned to anyone
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-06-13 07:23 UTC by Takaya Saito
Modified: 2012-01-04 10:11 UTC (History)
5 users (show)

See Also:
Host:
Target:
Build:
Known to work:
Known to fail:
Last reconfirmed: 2011-06-13 09:43:36


Attachments
Draft patch (400 bytes, patch)
2011-06-13 09:34 UTC, Paolo Carlini
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Takaya Saito 2011-06-13 07:23:12 UTC
testsuite ( bug.cc ) :

#include <iostream> // any C++ header not including <algorithm>

#define max( a, b ) bad_macro
#include <algorithm>


output:

In file included from /usr/local/gcc47-20110604/lib/gcc/i686-pc-cygwin/4.7.0/../../../../include/c++/4.7.0/bits/stl_algo.h:62:0,
                 from /usr/local/gcc47-20110604/lib/gcc/i686-pc-cygwin/4.7.0/../../../../include/c++/4.7.0/algorithm:63,
                 from bug.cc:4:
/usr/local/gcc47-20110604/lib/gcc/i686-pc-cygwin/4.7.0/../../../../include/c++/4.7.0/bits/algorithmfwd.h:358:41: error: macro "max" passed 3 arguments, but takes just 2
/usr/local/gcc47-20110604/lib/gcc/i686-pc-cygwin/4.7.0/../../../../include/c++/4.7.0/bits/algorithmfwd.h:354:5: error: template declaration of 'const _Tp& std::bad_macro'
/usr/local/gcc47-20110604/lib/gcc/i686-pc-cygwin/4.7.0/../../../../include/c++/4.7.0/bits/algorithmfwd.h:358:5: error: template declaration of 'const _Tp& std::max'


note:

The same error occurs in case of including <limits>.
Comment 1 Chris Jefferson 2011-06-13 07:55:33 UTC
max is a term defiend in the standard library. It is undefined behaviour if you #define it to something else when you are using standard library headers. Don't do that :)
Comment 2 Takaya Saito 2011-06-13 08:09:17 UTC
(In reply to comment #1)
> max is a term defiend in the standard library. It is undefined behaviour if you
> #define it to something else when you are using standard library headers. Don't
> do that :)

Yes, that's right.  But <windows.h> does it.
Comment 3 Chris Jefferson 2011-06-13 08:15:39 UTC
Ah yes. This is unfortunate, and I believe tricky to fix at the gcc end. We could in principle add '#undef min, #undef max', but I worry that might break something else.

If you '#define NOMINMAX' before including windows.h, that should stop the declarations, although it can break some windows libraries. The other option is to do:

#include <windows.h>
#ifdef min
#undef min
#endif
#ifdef max
#undef max
#endif
Comment 4 Takaya Saito 2011-06-13 08:29:15 UTC
(In reply to comment #3)
> Ah yes. This is unfortunate, and I believe tricky to fix at the gcc end. We
> could in principle add '#undef min, #undef max', but I worry that might break
> something else.
> 
> If you '#define NOMINMAX' before including windows.h, that should stop the
> declarations, although it can break some windows libraries. The other option is
> to do:
> 
> #include <windows.h>
> #ifdef min
> #undef min
> #endif
> #ifdef max
> #undef max
> #endif


Well, this code:


// #include <iostream>

#define max( a, b ) bad_macro
#include <algorithm> // OK


does not raise errors, because there exist "#undef min" and "#undef max"
in file <bits/c++config>.  So I think it should be a bug.
Comment 5 Paolo Carlini 2011-06-13 09:25:33 UTC
Note that <iostream>, before anything else, does #include <bits/c++config>, thus there is something weird going on. If you can spot it, just tell me and let's resolve this stupid M$ thing.
Comment 6 Paolo Carlini 2011-06-13 09:29:07 UTC
Oh yes, the issue of course is that the #undef themselves are inside the include guards of c++config, thus happen only once. We can take them out and "solve" the issue.
Comment 7 Paolo Carlini 2011-06-13 09:34:52 UTC
Created attachment 24506 [details]
Draft patch

This is what I mean. It works of course, but in my opinion is even more ugly than what we had already place for this M$ insanity. Before applying I'm going to post it to the mailing list and see if there are no objections.
Comment 8 Paolo Carlini 2011-06-13 09:43:36 UTC
Let's ask Dave's opinion on this. A possibility I can see is doing the dirty undefs only when _WIN32, __MINGW32__ & co are defined.
Comment 9 Jakub Jelinek 2011-06-13 10:57:46 UTC
Even better insert it into the c++config.h header on the broken targets.
The problem with undefining etc. anything outside of the guards means that
for this very often used header suddenly the guards won't be usable by the preprocessor, so it will have to read and parse the header again and again.
And the header is quite largish (> 40KB), so it will make a difference.
Comment 10 Paolo Carlini 2011-06-13 11:36:50 UTC
I agree in principle about the "c++config.h" (aka os_defines.h) header of the broken targets (which would be, I suppose, djgpp and mingw32) and considered using it earlier today, but at the moment it's included inside the guards of c++config itself (and has its own guards). What shall we do about that?
Comment 11 Paolo Carlini 2011-06-13 11:54:27 UTC
To be clear: if we want to close this as WONTFIX, I'm not objecting. I'd like only to ear from Dave, though, because I'm not in touch with anybody seriously using GCC on the affected targets.
Comment 12 Paolo Carlini 2011-06-26 22:39:20 UTC
Dave, do you have an opinion about this issue?

I'm tempted to close it as WONTFIX, frankly, I don't think we want to go beyond the #undefs we already have got in c++config, I would even do less (but probably code is already relying on that),
Comment 13 Paolo Carlini 2012-01-04 10:11:29 UTC
Closing.