[PATCH] D33080: [Libc++] Use #pragma push_macro/pop_macro to better handle min/max on Windows
This revision was automatically updated to reflect the committed changes. Closed by commit rL304357: [Libc++] Use #pragma push_macro/pop_macro to better handle min/max on Windows (authored by EricWF). Changed prior to commit: https://reviews.llvm.org/D33080?vs=100361=100925#toc Repository: rL LLVM https://reviews.llvm.org/D33080 Files: libcxx/trunk/include/__bit_reference libcxx/trunk/include/__config libcxx/trunk/include/__hash_table libcxx/trunk/include/__mutex_base libcxx/trunk/include/__split_buffer libcxx/trunk/include/__std_stream libcxx/trunk/include/__string libcxx/trunk/include/__threading_support libcxx/trunk/include/__tree libcxx/trunk/include/__undef_macros libcxx/trunk/include/__undef_min_max libcxx/trunk/include/algorithm libcxx/trunk/include/array libcxx/trunk/include/bitset libcxx/trunk/include/chrono libcxx/trunk/include/deque libcxx/trunk/include/experimental/algorithm libcxx/trunk/include/experimental/dynarray libcxx/trunk/include/experimental/functional libcxx/trunk/include/experimental/memory_resource libcxx/trunk/include/experimental/numeric libcxx/trunk/include/experimental/optional libcxx/trunk/include/experimental/string_view libcxx/trunk/include/forward_list libcxx/trunk/include/fstream libcxx/trunk/include/istream libcxx/trunk/include/limits libcxx/trunk/include/list libcxx/trunk/include/locale libcxx/trunk/include/memory libcxx/trunk/include/module.modulemap libcxx/trunk/include/mutex libcxx/trunk/include/numeric libcxx/trunk/include/optional libcxx/trunk/include/random libcxx/trunk/include/ratio libcxx/trunk/include/regex libcxx/trunk/include/shared_mutex libcxx/trunk/include/sstream libcxx/trunk/include/streambuf libcxx/trunk/include/string libcxx/trunk/include/string_view libcxx/trunk/include/thread libcxx/trunk/include/valarray libcxx/trunk/include/vector libcxx/trunk/src/condition_variable.cpp libcxx/trunk/src/ios.cpp libcxx/trunk/src/locale.cpp libcxx/trunk/src/mutex.cpp libcxx/trunk/src/strstream.cpp libcxx/trunk/test/libcxx/min_max_macros.sh.cpp Index: libcxx/trunk/src/strstream.cpp === --- libcxx/trunk/src/strstream.cpp +++ libcxx/trunk/src/strstream.cpp @@ -13,6 +13,7 @@ #include "cstring" #include "cstdlib" #include "__debug" +#include "__undef_macros" _LIBCPP_BEGIN_NAMESPACE_STD Index: libcxx/trunk/src/locale.cpp === --- libcxx/trunk/src/locale.cpp +++ libcxx/trunk/src/locale.cpp @@ -36,6 +36,7 @@ #endif #include #include +#include "__undef_macros" // On Linux, wint_t and wchar_t have different signed-ness, and this causes // lots of noise in the build log, but no bugs that I know of. Index: libcxx/trunk/src/mutex.cpp === --- libcxx/trunk/src/mutex.cpp +++ libcxx/trunk/src/mutex.cpp @@ -12,6 +12,7 @@ #include "limits" #include "system_error" #include "include/atomic_support.h" +#include "__undef_macros" _LIBCPP_BEGIN_NAMESPACE_STD #ifndef _LIBCPP_HAS_NO_THREADS Index: libcxx/trunk/src/condition_variable.cpp === --- libcxx/trunk/src/condition_variable.cpp +++ libcxx/trunk/src/condition_variable.cpp @@ -14,6 +14,7 @@ #include "condition_variable" #include "thread" #include "system_error" +#include "__undef_macros" _LIBCPP_BEGIN_NAMESPACE_STD Index: libcxx/trunk/src/ios.cpp === --- libcxx/trunk/src/ios.cpp +++ libcxx/trunk/src/ios.cpp @@ -22,6 +22,7 @@ #include "new" #include "streambuf" #include "string" +#include "__undef_macros" _LIBCPP_BEGIN_NAMESPACE_STD Index: libcxx/trunk/include/sstream === --- libcxx/trunk/include/sstream +++ libcxx/trunk/include/sstream @@ -175,12 +175,14 @@ #include #include -#include <__undef_min_max> - #if !defined(_LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER) #pragma GCC system_header #endif +_LIBCPP_PUSH_MACROS +#include <__undef_macros> + + _LIBCPP_BEGIN_NAMESPACE_STD // basic_stringbuf @@ -970,4 +972,6 @@ _LIBCPP_END_NAMESPACE_STD +_LIBCPP_POP_MACROS + #endif // _LIBCPP_SSTREAM Index: libcxx/trunk/include/string === --- libcxx/trunk/include/string +++ libcxx/trunk/include/string @@ -484,14 +484,16 @@ #include #endif -#include <__undef_min_max> - #include <__debug> #if !defined(_LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER) #pragma GCC system_header #endif +_LIBCPP_PUSH_MACROS +#include <__undef_macros> + + _LIBCPP_BEGIN_NAMESPACE_STD // fpos @@ -4041,4 +4043,6 @@ _LIBCPP_END_NAMESPACE_STD +_LIBCPP_POP_MACROS + #endif // _LIBCPP_STRING Index: libcxx/trunk/include/mutex
[PATCH] D33080: [Libc++] Use #pragma push_macro/pop_macro to better handle min/max on Windows
EricWF accepted this revision. EricWF added a comment. LGTM. https://reviews.llvm.org/D33080 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D33080: [Libc++] Use #pragma push_macro/pop_macro to better handle min/max on Windows
EricWF updated this revision to Diff 100361. EricWF added a comment. - Fix whitespace in `__config`. @compnerd Any final thoughts? https://reviews.llvm.org/D33080 Files: include/__bit_reference include/__config include/__hash_table include/__mutex_base include/__split_buffer include/__std_stream include/__string include/__threading_support include/__tree include/__undef_macros include/__undef_min_max include/algorithm include/array include/bitset include/chrono include/deque include/experimental/algorithm include/experimental/dynarray include/experimental/functional include/experimental/memory_resource include/experimental/numeric include/experimental/optional include/experimental/string_view include/forward_list include/fstream include/istream include/limits include/list include/locale include/memory include/module.modulemap include/mutex include/numeric include/optional include/random include/ratio include/regex include/shared_mutex include/sstream include/streambuf include/string include/string_view include/thread include/valarray include/vector src/condition_variable.cpp src/ios.cpp src/locale.cpp src/mutex.cpp src/strstream.cpp test/libcxx/min_max_macros.sh.cpp Index: test/libcxx/min_max_macros.sh.cpp === --- /dev/null +++ test/libcxx/min_max_macros.sh.cpp @@ -0,0 +1,298 @@ +// -*- C++ -*- +//===--===// +// +// The LLVM Compiler Infrastructure +// +// This file is dual licensed under the MIT and the University of Illinois Open +// Source Licenses. See LICENSE.TXT for details. +// +//===--===// + +// Test that we can include each header in two TU's and link them together. + +// RUN: %compile -fsyntax-only + +// Prevent from generating deprecated warnings for this test. +#if defined(__DEPRECATED) +#undef __DEPRECATED +#endif + +#define TEST_MACROS() static_assert(min() == true && max() == true, "") +#define min() true +#define max() true + +// Top level headers +#include +TEST_MACROS(); +#include +TEST_MACROS(); +#include +TEST_MACROS(); +#ifndef _LIBCPP_HAS_NO_THREADS +#include +TEST_MACROS(); +#endif +#include +TEST_MACROS(); +#include +TEST_MACROS(); +#include +TEST_MACROS(); +#include +TEST_MACROS(); +#include +TEST_MACROS(); +#include +TEST_MACROS(); +#include +TEST_MACROS(); +#include +TEST_MACROS(); +#include +TEST_MACROS(); +#include +TEST_MACROS(); +#include +TEST_MACROS(); +#include +TEST_MACROS(); +#include +TEST_MACROS(); +#include +TEST_MACROS(); +#include +TEST_MACROS(); +#include +TEST_MACROS(); +#include +TEST_MACROS(); +#include +TEST_MACROS(); +#include +TEST_MACROS(); +#include +TEST_MACROS(); +#include +TEST_MACROS(); +#include +TEST_MACROS(); +#include +TEST_MACROS(); +#include +TEST_MACROS(); +#include +TEST_MACROS(); +#include +TEST_MACROS(); +#include +TEST_MACROS(); +#include +TEST_MACROS(); +#include +TEST_MACROS(); +#include +TEST_MACROS(); +#include +TEST_MACROS(); +#include +TEST_MACROS(); +#include +TEST_MACROS(); +#include +TEST_MACROS(); +#include +TEST_MACROS(); +#include +TEST_MACROS(); +#include +TEST_MACROS(); +#include +TEST_MACROS(); +#ifndef _LIBCPP_HAS_NO_THREADS +#include +TEST_MACROS(); +#endif +#include +TEST_MACROS(); +#include +TEST_MACROS(); +#include +TEST_MACROS(); +#include +TEST_MACROS(); +#include +TEST_MACROS(); +#include +TEST_MACROS(); +#include +TEST_MACROS(); +#include +TEST_MACROS(); +#include +TEST_MACROS(); +#include +TEST_MACROS(); +#include +TEST_MACROS(); +#include +TEST_MACROS(); +#include +TEST_MACROS(); +#include +TEST_MACROS(); +#include +TEST_MACROS(); +#include +TEST_MACROS(); +#ifndef _LIBCPP_HAS_NO_THREADS +#include +TEST_MACROS(); +#endif +#include +TEST_MACROS(); +#include +TEST_MACROS(); +#include +TEST_MACROS(); +#include +TEST_MACROS(); +#include +TEST_MACROS(); +#include +TEST_MACROS(); +#include +TEST_MACROS(); +#include +TEST_MACROS(); +#include +TEST_MACROS(); +#include +TEST_MACROS(); +#include +TEST_MACROS(); +#ifndef _LIBCPP_HAS_NO_THREADS +#include +TEST_MACROS(); +#endif +#include +TEST_MACROS(); +#include +TEST_MACROS(); +#include +TEST_MACROS(); +#include +TEST_MACROS(); +#include +TEST_MACROS(); +#include +TEST_MACROS(); +#include +TEST_MACROS(); +#include +TEST_MACROS(); +#include +TEST_MACROS(); +#include +TEST_MACROS(); +#include +TEST_MACROS(); +#include +TEST_MACROS(); +#include +TEST_MACROS(); +#include +TEST_MACROS(); +#include +TEST_MACROS(); +#ifndef _LIBCPP_HAS_NO_THREADS +#include +TEST_MACROS(); +#endif +#include +TEST_MACROS(); +#include +TEST_MACROS(); +#include +TEST_MACROS(); +#include +TEST_MACROS(); +#include +TEST_MACROS(); +#include +TEST_MACROS(); +#include +TEST_MACROS();
[PATCH] D33080: [Libc++] Use #pragma push_macro/pop_macro to better handle min/max on Windows
EricWF updated this revision to Diff 100269. EricWF added a comment. - Remove the external include guards for `__undef_macros` as requested by @compnerd. I couldn't come up with a easy test case that showed any notable difference having the external guards. https://reviews.llvm.org/D33080 Files: include/__bit_reference include/__config include/__hash_table include/__mutex_base include/__split_buffer include/__std_stream include/__string include/__threading_support include/__tree include/__undef_macros include/__undef_min_max include/algorithm include/array include/bitset include/chrono include/deque include/experimental/algorithm include/experimental/dynarray include/experimental/functional include/experimental/memory_resource include/experimental/numeric include/experimental/optional include/experimental/string_view include/forward_list include/fstream include/istream include/limits include/list include/locale include/memory include/module.modulemap include/mutex include/numeric include/optional include/random include/ratio include/regex include/shared_mutex include/sstream include/streambuf include/string include/string_view include/thread include/valarray include/vector src/condition_variable.cpp src/ios.cpp src/locale.cpp src/mutex.cpp src/strstream.cpp test/libcxx/min_max_macros.sh.cpp Index: test/libcxx/min_max_macros.sh.cpp === --- /dev/null +++ test/libcxx/min_max_macros.sh.cpp @@ -0,0 +1,298 @@ +// -*- C++ -*- +//===--===// +// +// The LLVM Compiler Infrastructure +// +// This file is dual licensed under the MIT and the University of Illinois Open +// Source Licenses. See LICENSE.TXT for details. +// +//===--===// + +// Test that we can include each header in two TU's and link them together. + +// RUN: %compile -fsyntax-only + +// Prevent from generating deprecated warnings for this test. +#if defined(__DEPRECATED) +#undef __DEPRECATED +#endif + +#define TEST_MACROS() static_assert(min() == true && max() == true, "") +#define min() true +#define max() true + +// Top level headers +#include +TEST_MACROS(); +#include +TEST_MACROS(); +#include +TEST_MACROS(); +#ifndef _LIBCPP_HAS_NO_THREADS +#include +TEST_MACROS(); +#endif +#include +TEST_MACROS(); +#include +TEST_MACROS(); +#include +TEST_MACROS(); +#include +TEST_MACROS(); +#include +TEST_MACROS(); +#include +TEST_MACROS(); +#include +TEST_MACROS(); +#include +TEST_MACROS(); +#include +TEST_MACROS(); +#include +TEST_MACROS(); +#include +TEST_MACROS(); +#include +TEST_MACROS(); +#include +TEST_MACROS(); +#include +TEST_MACROS(); +#include +TEST_MACROS(); +#include +TEST_MACROS(); +#include +TEST_MACROS(); +#include +TEST_MACROS(); +#include +TEST_MACROS(); +#include +TEST_MACROS(); +#include +TEST_MACROS(); +#include +TEST_MACROS(); +#include +TEST_MACROS(); +#include +TEST_MACROS(); +#include +TEST_MACROS(); +#include +TEST_MACROS(); +#include +TEST_MACROS(); +#include +TEST_MACROS(); +#include +TEST_MACROS(); +#include +TEST_MACROS(); +#include +TEST_MACROS(); +#include +TEST_MACROS(); +#include +TEST_MACROS(); +#include +TEST_MACROS(); +#include +TEST_MACROS(); +#include +TEST_MACROS(); +#include +TEST_MACROS(); +#include +TEST_MACROS(); +#ifndef _LIBCPP_HAS_NO_THREADS +#include +TEST_MACROS(); +#endif +#include +TEST_MACROS(); +#include +TEST_MACROS(); +#include +TEST_MACROS(); +#include +TEST_MACROS(); +#include +TEST_MACROS(); +#include +TEST_MACROS(); +#include +TEST_MACROS(); +#include +TEST_MACROS(); +#include +TEST_MACROS(); +#include +TEST_MACROS(); +#include +TEST_MACROS(); +#include +TEST_MACROS(); +#include +TEST_MACROS(); +#include +TEST_MACROS(); +#include +TEST_MACROS(); +#include +TEST_MACROS(); +#ifndef _LIBCPP_HAS_NO_THREADS +#include +TEST_MACROS(); +#endif +#include +TEST_MACROS(); +#include +TEST_MACROS(); +#include +TEST_MACROS(); +#include +TEST_MACROS(); +#include +TEST_MACROS(); +#include +TEST_MACROS(); +#include +TEST_MACROS(); +#include +TEST_MACROS(); +#include +TEST_MACROS(); +#include +TEST_MACROS(); +#include +TEST_MACROS(); +#ifndef _LIBCPP_HAS_NO_THREADS +#include +TEST_MACROS(); +#endif +#include +TEST_MACROS(); +#include +TEST_MACROS(); +#include +TEST_MACROS(); +#include +TEST_MACROS(); +#include +TEST_MACROS(); +#include +TEST_MACROS(); +#include +TEST_MACROS(); +#include +TEST_MACROS(); +#include +TEST_MACROS(); +#include +TEST_MACROS(); +#include +TEST_MACROS(); +#include +TEST_MACROS(); +#include +TEST_MACROS(); +#include +TEST_MACROS(); +#include +TEST_MACROS(); +#ifndef _LIBCPP_HAS_NO_THREADS +#include +TEST_MACROS(); +#endif +#include +TEST_MACROS(); +#include +TEST_MACROS(); +#include
[PATCH] D33080: [Libc++] Use #pragma push_macro/pop_macro to better handle min/max on Windows
EricWF added a comment. In https://reviews.llvm.org/D33080#754442, @compnerd wrote: > I think that we should sink the `min`/`max` checks into `__undef_macros`. I > don't like the idea of littering that check everywhere. I would much rather litter at the cost of the implementation than needlessly include `__undef_macros` in every header; Especially because it's almost always unneeded. I realize it's ugly internally but I think it's worth it. I'll attempt to write some compile time tests to ensure I'm not making a mountain out of a molehill but could you also re-consider @compnerd? https://reviews.llvm.org/D33080 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D33080: [Libc++] Use #pragma push_macro/pop_macro to better handle min/max on Windows
compnerd requested changes to this revision.
compnerd added a comment.
This revision now requires changes to proceed.
I think that we should sink the `min`/`max` checks into `__undef_macros`. I
don't like the idea of littering that check everywhere.
Comment at: include/__config:1218
+# if defined(_LIBCPP_COMPILER_MSVC)
+# define _LIBCPP_PUSH_MACROS \
+ __pragma(push_macro("min")) \
clang-format these please? It does a nice job of aligning the `\`.
https://reviews.llvm.org/D33080
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D33080: [Libc++] Use #pragma push_macro/pop_macro to better handle min/max on Windows
bcraig added a comment. > I noticed that the Windows STL headers have to do this dance with `new` (even > though they do `(foo)(...)` for `min` and `max`). If we're going to need > to guard against a bunch of macros I would like to use a single approach. > Other than updating the `#if defined(min) || defined(max)` lines it's trivial > to guard > against additional macros. I think the guarding of 'new' is done because of user code, not because of headers #defining new. There is a lot of old Windows code out there that would define 'new' to something else in order to get extra instrumentation. I want to say this was even done in some of the MFC example 'project templates' One of the things I don't like about push / pop macro is that it isn't the same as doing nothing with the macro. If something between the push and pop specifically wanted to use or modify the macro, the pop macro will destroy that state. Granted, in this situation, that seems both unlikely, and it would be evil heaped upon evil. With push and pop macro, you also need to be very careful about how things are positioned. The push and undef need to happen after all the other includes in the header are processed, and care needs to be taken not to include anything offensive within the bounds of the push and pop. You also need to remember to do the pop. push and pop macro are acceptable. I still have a preference for the parenthesis approach though. Comment at: include/shared_mutex:128-136 +_LIBCPP_PUSH_MACROS +#if defined(min) || defined(max) +# include <__undef_macros> +#endif + + #if _LIBCPP_STD_VER > 11 || defined(_LIBCPP_BUILDING_SHARED_MUTEX) Shouldn't the push macro be somewhere below this potential include? https://reviews.llvm.org/D33080 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D33080: [Libc++] Use #pragma push_macro/pop_macro to better handle min/max on Windows
EricWF updated this revision to Diff 98725. EricWF added a comment. - Fix missing `_LIBCPP_POP_MACROS` in `<__threading_support>`. - Add test that each header is unaffected by `min` and `max` macros and that the macros themselves are unaffected. https://reviews.llvm.org/D33080 Files: include/__bit_reference include/__config include/__hash_table include/__mutex_base include/__split_buffer include/__std_stream include/__string include/__threading_support include/__tree include/__undef_macros include/__undef_min_max include/algorithm include/array include/bitset include/chrono include/deque include/experimental/algorithm include/experimental/dynarray include/experimental/functional include/experimental/memory_resource include/experimental/numeric include/experimental/optional include/experimental/string_view include/forward_list include/fstream include/istream include/limits include/list include/locale include/memory include/module.modulemap include/mutex include/numeric include/optional include/random include/ratio include/regex include/shared_mutex include/sstream include/streambuf include/string include/string_view include/thread include/valarray include/vector test/libcxx/min_max_macros.sh.cpp Index: test/libcxx/min_max_macros.sh.cpp === --- /dev/null +++ test/libcxx/min_max_macros.sh.cpp @@ -0,0 +1,298 @@ +// -*- C++ -*- +//===--===// +// +// The LLVM Compiler Infrastructure +// +// This file is dual licensed under the MIT and the University of Illinois Open +// Source Licenses. See LICENSE.TXT for details. +// +//===--===// + +// Test that we can include each header in two TU's and link them together. + +// RUN: %compile -fsyntax-only + +// Prevent from generating deprecated warnings for this test. +#if defined(__DEPRECATED) +#undef __DEPRECATED +#endif + +#define TEST_MACROS() static_assert(min() == true && max() == true, "") +#define min() true +#define max() true + +// Top level headers +#include +TEST_MACROS(); +#include +TEST_MACROS(); +#include +TEST_MACROS(); +#ifndef _LIBCPP_HAS_NO_THREADS +#include +TEST_MACROS(); +#endif +#include +TEST_MACROS(); +#include +TEST_MACROS(); +#include +TEST_MACROS(); +#include +TEST_MACROS(); +#include +TEST_MACROS(); +#include +TEST_MACROS(); +#include +TEST_MACROS(); +#include +TEST_MACROS(); +#include +TEST_MACROS(); +#include +TEST_MACROS(); +#include +TEST_MACROS(); +#include +TEST_MACROS(); +#include +TEST_MACROS(); +#include +TEST_MACROS(); +#include +TEST_MACROS(); +#include +TEST_MACROS(); +#include +TEST_MACROS(); +#include +TEST_MACROS(); +#include +TEST_MACROS(); +#include +TEST_MACROS(); +#include +TEST_MACROS(); +#include +TEST_MACROS(); +#include +TEST_MACROS(); +#include +TEST_MACROS(); +#include +TEST_MACROS(); +#include +TEST_MACROS(); +#include +TEST_MACROS(); +#include +TEST_MACROS(); +#include +TEST_MACROS(); +#include +TEST_MACROS(); +#include +TEST_MACROS(); +#include +TEST_MACROS(); +#include +TEST_MACROS(); +#include +TEST_MACROS(); +#include +TEST_MACROS(); +#include +TEST_MACROS(); +#include +TEST_MACROS(); +#include +TEST_MACROS(); +#ifndef _LIBCPP_HAS_NO_THREADS +#include +TEST_MACROS(); +#endif +#include +TEST_MACROS(); +#include +TEST_MACROS(); +#include +TEST_MACROS(); +#include +TEST_MACROS(); +#include +TEST_MACROS(); +#include +TEST_MACROS(); +#include +TEST_MACROS(); +#include +TEST_MACROS(); +#include +TEST_MACROS(); +#include +TEST_MACROS(); +#include +TEST_MACROS(); +#include +TEST_MACROS(); +#include +TEST_MACROS(); +#include +TEST_MACROS(); +#include +TEST_MACROS(); +#include +TEST_MACROS(); +#ifndef _LIBCPP_HAS_NO_THREADS +#include +TEST_MACROS(); +#endif +#include +TEST_MACROS(); +#include +TEST_MACROS(); +#include +TEST_MACROS(); +#include +TEST_MACROS(); +#include +TEST_MACROS(); +#include +TEST_MACROS(); +#include +TEST_MACROS(); +#include +TEST_MACROS(); +#include +TEST_MACROS(); +#include +TEST_MACROS(); +#include +TEST_MACROS(); +#ifndef _LIBCPP_HAS_NO_THREADS +#include +TEST_MACROS(); +#endif +#include +TEST_MACROS(); +#include +TEST_MACROS(); +#include +TEST_MACROS(); +#include +TEST_MACROS(); +#include +TEST_MACROS(); +#include +TEST_MACROS(); +#include +TEST_MACROS(); +#include +TEST_MACROS(); +#include +TEST_MACROS(); +#include +TEST_MACROS(); +#include +TEST_MACROS(); +#include +TEST_MACROS(); +#include +TEST_MACROS(); +#include +TEST_MACROS(); +#include +TEST_MACROS(); +#ifndef _LIBCPP_HAS_NO_THREADS +#include +TEST_MACROS(); +#endif +#include +TEST_MACROS(); +#include +TEST_MACROS(); +#include +TEST_MACROS(); +#include +TEST_MACROS(); +#include +TEST_MACROS(); +#include +TEST_MACROS(); +#include
[PATCH] D33080: [Libc++] Use #pragma push_macro/pop_macro to better handle min/max on Windows
EricWF added a comment. In https://reviews.llvm.org/D33080#752202, @bcraig wrote: > I like the warning that you generate for min and max macros existing. Only on warn on platforms where we don't have `#pragma push_macro/pop_macro`. > Is the push_macro / pop_macro the right way to go though? You could throw > extra parenthesis around the declaration and usages of min/max to avoid macro > expansion. I noticed that the Windows STL headers have to do this dance with `new` (even though they do `(foo)(...)` for `min` and `max`). If we're going to need to guard against a bunch of macros I would like to use a single approach. Other than updating the `#if defined(min) || defined(max)` lines it's trivial to guard against additional macros. Also there are a lot of call sites and declarations for `min` and `max`. I think this approach is less invasive and more consistent. We're obviously not going to use `(foo)(...)` syntax everywhere, so lets use it nowhere. https://reviews.llvm.org/D33080 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D33080: [Libc++] Use #pragma push_macro/pop_macro to better handle min/max on Windows
bcraig added a comment.
I like the warning that you generate for min and max macros existing. Is the
push_macro / pop_macro the right way to go though? You could throw extra
parenthesis around the declaration and usages of min/max to avoid macro
expansion.
#define add(x,y) (x)-(y)
template
T (add)(T x, T y) // parens around add to suppress macro
{
return x+y;
}
int main() {
return
(add)(5, 4) + // uses function
add(5, 4);// uses macro
}
https://reviews.llvm.org/D33080
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D33080: [Libc++] Use #pragma push_macro/pop_macro to better handle min/max on Windows
EricWF added inline comments. Comment at: include/__threading_support:632 #endif // _LIBCPP_HAS_THREAD_API_PTHREAD #endif // !_LIBCPP_HAS_THREAD_LIBRARY_EXTERNAL || _LIBCPP_BUILDING_THREAD_LIBRARY_EXTERNAL Missing _LIBCPP_POP_MACROS https://reviews.llvm.org/D33080 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D33080: [Libc++] Use #pragma push_macro/pop_macro to better handle min/max on Windows
EricWF updated this revision to Diff 98562. EricWF added a comment. - Fix unterminated preprocessor directive. https://reviews.llvm.org/D33080 Files: include/__bit_reference include/__config include/__hash_table include/__mutex_base include/__split_buffer include/__std_stream include/__string include/__threading_support include/__tree include/__undef_macros include/__undef_min_max include/algorithm include/array include/bitset include/chrono include/deque include/experimental/algorithm include/experimental/functional include/experimental/optional include/forward_list include/fstream include/istream include/limits include/list include/locale include/memory include/module.modulemap include/mutex include/optional include/random include/ratio include/regex include/shared_mutex include/sstream include/string include/string_view include/valarray include/vector Index: include/vector === --- include/vector +++ include/vector @@ -275,14 +275,18 @@ #include <__split_buffer> #include <__functional_base> -#include <__undef_min_max> - #include <__debug> #if !defined(_LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER) #pragma GCC system_header #endif +_LIBCPP_PUSH_MACROS +#if defined(min) || defined(max) +# include <__undef_macros> +#endif + + _LIBCPP_BEGIN_NAMESPACE_STD template @@ -3357,4 +3361,6 @@ _LIBCPP_END_NAMESPACE_STD +_LIBCPP_POP_MACROS + #endif // _LIBCPP_VECTOR Index: include/valarray === --- include/valarray +++ include/valarray @@ -347,12 +347,16 @@ #include #include -#include <__undef_min_max> - #if !defined(_LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER) #pragma GCC system_header #endif +_LIBCPP_PUSH_MACROS +#if defined(min) || defined(max) +# include <__undef_macros> +#endif + + _LIBCPP_BEGIN_NAMESPACE_STD template class _LIBCPP_TEMPLATE_VIS valarray; @@ -4865,4 +4869,6 @@ _LIBCPP_END_NAMESPACE_STD +_LIBCPP_POP_MACROS + #endif // _LIBCPP_VALARRAY Index: include/string_view === --- include/string_view +++ include/string_view @@ -167,7 +167,6 @@ */ #include <__config> - #include <__string> #include #include @@ -179,6 +178,12 @@ #pragma GCC system_header #endif +_LIBCPP_PUSH_MACROS +#if defined(min) || defined(max) +# include <__undef_macros> +#endif + + _LIBCPP_BEGIN_NAMESPACE_STD template > @@ -792,4 +797,6 @@ #endif _LIBCPP_END_NAMESPACE_STD +_LIBCPP_POP_MACROS + #endif // _LIBCPP_STRING_VIEW Index: include/string === --- include/string +++ include/string @@ -484,14 +484,18 @@ #include #endif -#include <__undef_min_max> - #include <__debug> #if !defined(_LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER) #pragma GCC system_header #endif +_LIBCPP_PUSH_MACROS +#if defined(min) || defined(max) +# include <__undef_macros> +#endif + + _LIBCPP_BEGIN_NAMESPACE_STD // fpos @@ -4041,4 +4045,6 @@ _LIBCPP_END_NAMESPACE_STD +_LIBCPP_POP_MACROS + #endif // _LIBCPP_STRING Index: include/sstream === --- include/sstream +++ include/sstream @@ -175,12 +175,16 @@ #include #include -#include <__undef_min_max> - #if !defined(_LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER) #pragma GCC system_header #endif +_LIBCPP_PUSH_MACROS +#if defined(min) || defined(max) +# include <__undef_macros> +#endif + + _LIBCPP_BEGIN_NAMESPACE_STD // basic_stringbuf @@ -970,4 +974,6 @@ _LIBCPP_END_NAMESPACE_STD +_LIBCPP_POP_MACROS + #endif // _LIBCPP_SSTREAM Index: include/shared_mutex === --- include/shared_mutex +++ include/shared_mutex @@ -125,12 +125,16 @@ #include <__config> +_LIBCPP_PUSH_MACROS +#if defined(min) || defined(max) +# include <__undef_macros> +#endif + + #if _LIBCPP_STD_VER > 11 || defined(_LIBCPP_BUILDING_SHARED_MUTEX) #include <__mutex_base> -#include <__undef_min_max> - #if !defined(_LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER) #pragma GCC system_header #endif @@ -500,4 +504,6 @@ #endif // _LIBCPP_STD_VER > 11 +_LIBCPP_POP_MACROS + #endif // _LIBCPP_SHARED_MUTEX Index: include/regex === --- include/regex +++ include/regex @@ -765,12 +765,16 @@ #include #include -#include <__undef_min_max> - #if !defined(_LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER) #pragma GCC system_header #endif +_LIBCPP_PUSH_MACROS +#if defined(min) || defined(max) +# include <__undef_macros> +#endif + + _LIBCPP_BEGIN_NAMESPACE_STD namespace regex_constants @@ -6562,4 +6566,6 @@ _LIBCPP_END_NAMESPACE_STD +_LIBCPP_POP_MACROS + #endif // _LIBCPP_REGEX Index: include/ratio === ---
[PATCH] D33080: [Libc++] Use #pragma push_macro/pop_macro to better handle min/max on Windows
EricWF updated this revision to Diff 98561. EricWF added a comment. - Remove failing test. This patch cannot be easily tested because the system's libc might also mess with the definitions of `min` and `max`. https://reviews.llvm.org/D33080 Files: include/__bit_reference include/__config include/__hash_table include/__mutex_base include/__split_buffer include/__std_stream include/__string include/__threading_support include/__tree include/__undef_macros include/__undef_min_max include/algorithm include/array include/bitset include/chrono include/deque include/experimental/algorithm include/experimental/functional include/experimental/optional include/forward_list include/fstream include/istream include/limits include/list include/locale include/memory include/module.modulemap include/mutex include/optional include/random include/ratio include/regex include/shared_mutex include/sstream include/string include/string_view include/valarray include/vector Index: include/vector === --- include/vector +++ include/vector @@ -275,14 +275,18 @@ #include <__split_buffer> #include <__functional_base> -#include <__undef_min_max> - #include <__debug> #if !defined(_LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER) #pragma GCC system_header #endif +_LIBCPP_PUSH_MACROS +#if defined(min) || defined(max) +# include <__undef_macros> +#endif + + _LIBCPP_BEGIN_NAMESPACE_STD template @@ -3357,4 +3361,6 @@ _LIBCPP_END_NAMESPACE_STD +_LIBCPP_POP_MACROS + #endif // _LIBCPP_VECTOR Index: include/valarray === --- include/valarray +++ include/valarray @@ -347,12 +347,16 @@ #include #include -#include <__undef_min_max> - #if !defined(_LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER) #pragma GCC system_header #endif +_LIBCPP_PUSH_MACROS +#if defined(min) || defined(max) +# include <__undef_macros> +#endif + + _LIBCPP_BEGIN_NAMESPACE_STD template class _LIBCPP_TEMPLATE_VIS valarray; @@ -4865,4 +4869,6 @@ _LIBCPP_END_NAMESPACE_STD +_LIBCPP_POP_MACROS + #endif // _LIBCPP_VALARRAY Index: include/string_view === --- include/string_view +++ include/string_view @@ -167,7 +167,6 @@ */ #include <__config> - #include <__string> #include #include @@ -179,6 +178,12 @@ #pragma GCC system_header #endif +_LIBCPP_PUSH_MACROS +#if defined(min) || defined(max) +# include <__undef_macros> +#endif + + _LIBCPP_BEGIN_NAMESPACE_STD template > @@ -792,4 +797,6 @@ #endif _LIBCPP_END_NAMESPACE_STD +_LIBCPP_POP_MACROS + #endif // _LIBCPP_STRING_VIEW Index: include/string === --- include/string +++ include/string @@ -484,14 +484,18 @@ #include #endif -#include <__undef_min_max> - #include <__debug> #if !defined(_LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER) #pragma GCC system_header #endif +_LIBCPP_PUSH_MACROS +#if defined(min) || defined(max) +# include <__undef_macros> +#endif + + _LIBCPP_BEGIN_NAMESPACE_STD // fpos @@ -4041,4 +4045,6 @@ _LIBCPP_END_NAMESPACE_STD +_LIBCPP_POP_MACROS + #endif // _LIBCPP_STRING Index: include/sstream === --- include/sstream +++ include/sstream @@ -175,12 +175,16 @@ #include #include -#include <__undef_min_max> - #if !defined(_LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER) #pragma GCC system_header #endif +_LIBCPP_PUSH_MACROS +#if defined(min) || defined(max) +# include <__undef_macros> +#endif + + _LIBCPP_BEGIN_NAMESPACE_STD // basic_stringbuf @@ -970,4 +974,6 @@ _LIBCPP_END_NAMESPACE_STD +_LIBCPP_POP_MACROS + #endif // _LIBCPP_SSTREAM Index: include/shared_mutex === --- include/shared_mutex +++ include/shared_mutex @@ -125,12 +125,16 @@ #include <__config> +_LIBCPP_PUSH_MACROS +#if defined(min) || defined(max) +# include <__undef_macros> +#endif + + #if _LIBCPP_STD_VER > 11 || defined(_LIBCPP_BUILDING_SHARED_MUTEX) #include <__mutex_base> -#include <__undef_min_max> - #if !defined(_LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER) #pragma GCC system_header #endif @@ -500,4 +504,6 @@ #endif // _LIBCPP_STD_VER > 11 +_LIBCPP_POP_MACROS + #endif // _LIBCPP_SHARED_MUTEX Index: include/regex === --- include/regex +++ include/regex @@ -765,12 +765,16 @@ #include #include -#include <__undef_min_max> - #if !defined(_LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER) #pragma GCC system_header #endif +_LIBCPP_PUSH_MACROS +#if defined(min) || defined(max) +# include <__undef_macros> +#endif + + _LIBCPP_BEGIN_NAMESPACE_STD namespace regex_constants @@ -6562,4 +6566,6 @@ _LIBCPP_END_NAMESPACE_STD +_LIBCPP_POP_MACROS + #endif // _LIBCPP_REGEX
[PATCH] D33080: [Libc++] Use #pragma push_macro/pop_macro to better handle min/max on Windows
EricWF created this revision. Herald added a subscriber: krytarowski. This patch improves how libc++ handles min/max macros within the headers. Previously libc++ would undef them and emit a warning. This patch changes libc++ to use `#pragma push_macro` to save the macro before undefining it, and `#pragma pop_macro` to restore the macros and the end of the header. https://reviews.llvm.org/D33080 Files: include/__bit_reference include/__config include/__hash_table include/__mutex_base include/__split_buffer include/__std_stream include/__string include/__threading_support include/__tree include/__undef_macros include/__undef_min_max include/algorithm include/array include/bitset include/chrono include/deque include/experimental/algorithm include/experimental/functional include/experimental/optional include/forward_list include/fstream include/istream include/limits include/list include/locale include/memory include/module.modulemap include/mutex include/optional include/random include/ratio include/regex include/shared_mutex include/sstream include/string include/string_view include/valarray include/vector test/libcxx/min_max_macros.sh.cpp Index: test/libcxx/min_max_macros.sh.cpp === --- /dev/null +++ test/libcxx/min_max_macros.sh.cpp @@ -0,0 +1,295 @@ +// -*- C++ -*- +//===--===// +// +// The LLVM Compiler Infrastructure +// +// This file is dual licensed under the MIT and the University of Illinois Open +// Source Licenses. See LICENSE.TXT for details. +// +//===--===// + +// Test that we are not affected by min/max macro definitions + +// RUN: %compile -fsyntax-only + +#define min true +#define max true + +#define TEST_MIN_MAX() static_assert(min == true && max == true, "") + + +// Top level headers +#include +TEST_MIN_MAX(); +#include +TEST_MIN_MAX(); +#include +TEST_MIN_MAX(); +#ifndef _LIBCPP_HAS_NO_THREADS +#include +TEST_MIN_MAX(); +#endif +#include +TEST_MIN_MAX(); +#include +TEST_MIN_MAX(); +#include +TEST_MIN_MAX(); +#include +TEST_MIN_MAX(); +#include +TEST_MIN_MAX(); +#include +TEST_MIN_MAX(); +#include +TEST_MIN_MAX(); +#include +TEST_MIN_MAX(); +#include +TEST_MIN_MAX(); +#include +TEST_MIN_MAX(); +#include +TEST_MIN_MAX(); +#include +TEST_MIN_MAX(); +#include +TEST_MIN_MAX(); +#include +TEST_MIN_MAX(); +#include +TEST_MIN_MAX(); +#include +TEST_MIN_MAX(); +#include +TEST_MIN_MAX(); +#include +TEST_MIN_MAX(); +#include +TEST_MIN_MAX(); +#include +TEST_MIN_MAX(); +#include +TEST_MIN_MAX(); +#include +TEST_MIN_MAX(); +#include +TEST_MIN_MAX(); +#include +TEST_MIN_MAX(); +#include +TEST_MIN_MAX(); +#include +TEST_MIN_MAX(); +#include +TEST_MIN_MAX(); +#include +TEST_MIN_MAX(); +#include +TEST_MIN_MAX(); +#include +TEST_MIN_MAX(); +#include +TEST_MIN_MAX(); +#include +TEST_MIN_MAX(); +#include +TEST_MIN_MAX(); +#include +TEST_MIN_MAX(); +#include +TEST_MIN_MAX(); +#include +TEST_MIN_MAX(); +#include +TEST_MIN_MAX(); +#include +TEST_MIN_MAX(); +#ifndef _LIBCPP_HAS_NO_THREADS +#include +TEST_MIN_MAX(); +#endif +#include +TEST_MIN_MAX(); +#include +TEST_MIN_MAX(); +#include +TEST_MIN_MAX(); +#include +TEST_MIN_MAX(); +#include +TEST_MIN_MAX(); +#include +TEST_MIN_MAX(); +#include +TEST_MIN_MAX(); +#include +TEST_MIN_MAX(); +#include +TEST_MIN_MAX(); +#include +TEST_MIN_MAX(); +#include +TEST_MIN_MAX(); +#include +TEST_MIN_MAX(); +#include +TEST_MIN_MAX(); +#include +TEST_MIN_MAX(); +#include +TEST_MIN_MAX(); +#include +TEST_MIN_MAX(); +#ifndef _LIBCPP_HAS_NO_THREADS +#include +TEST_MIN_MAX(); +#endif +#include +TEST_MIN_MAX(); +#include +TEST_MIN_MAX(); +#include +TEST_MIN_MAX(); +#include +TEST_MIN_MAX(); +#include +TEST_MIN_MAX(); +#include +TEST_MIN_MAX(); +#include +TEST_MIN_MAX(); +#include +TEST_MIN_MAX(); +#include +TEST_MIN_MAX(); +#include +TEST_MIN_MAX(); +#include +TEST_MIN_MAX(); +#ifndef _LIBCPP_HAS_NO_THREADS +#include +TEST_MIN_MAX(); +#endif +#include +TEST_MIN_MAX(); +#include +TEST_MIN_MAX(); +#include +TEST_MIN_MAX(); +#include +TEST_MIN_MAX(); +#include +TEST_MIN_MAX(); +#include +TEST_MIN_MAX(); +#include +TEST_MIN_MAX(); +#include +TEST_MIN_MAX(); +#include +TEST_MIN_MAX(); +#include +TEST_MIN_MAX(); +#include +TEST_MIN_MAX(); +#include +TEST_MIN_MAX(); +#include +TEST_MIN_MAX(); +#include +TEST_MIN_MAX(); +#include +TEST_MIN_MAX(); +#ifndef _LIBCPP_HAS_NO_THREADS +#include +TEST_MIN_MAX(); +#endif +#include +TEST_MIN_MAX(); +#include +TEST_MIN_MAX(); +#include +TEST_MIN_MAX(); +#include +TEST_MIN_MAX(); +#include +TEST_MIN_MAX(); +#include +TEST_MIN_MAX(); +#include +TEST_MIN_MAX(); +#include +TEST_MIN_MAX(); +#include +TEST_MIN_MAX(); +#include +TEST_MIN_MAX(); +#include
