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

Reply via email to