search

boostorg / core

Boost Core Utilities
132 stars 86 forks source link

Order of disable/restore macros in boost/core/allocator_access.hpp #134

Closed joaquintides closed 1 year ago

joaquintides commented 1 year ago

The aforementioned file has the following macros for disabling and then restoring warnings:

#if defined(_LIBCPP_SUPPRESS_DEPRECATED_PUSH)
_LIBCPP_SUPPRESS_DEPRECATED_PUSH
#endif
#if defined(_STL_DISABLE_DEPRECATED_WARNING)
_STL_DISABLE_DEPRECATED_WARNING
#endif
#if defined(_MSC_VER)
#pragma warning(push)
#pragma warning(disable:4996)
#endif

...

#if defined(_LIBCPP_SUPPRESS_DEPRECATED_POP)
_LIBCPP_SUPPRESS_DEPRECATED_POP
#endif
#if defined(_STL_RESTORE_DEPRECATED_WARNING)
_STL_RESTORE_DEPRECATED_WARNING
#endif
#if defined(_MSC_VER)
#pragma warning(pop)
#endif

Shouldn't the restore macros be written in reverse order, or else shouldn't the macros be applied in an exclusive manner by way of #elifs?

glenfe commented 1 year ago

Reverse order sounds better. I'll take care of it for 1.82.0

joaquintides commented 1 year ago

IMHO #elifs are better because defined(_STL_RESTORE_DEPRECATED_WARNING) and defined(_MSC_VER) are not mutually exclusive, so you end up silencing the same warning twice.

glenfe commented 1 year ago

Addressed in 2286749f977e003f3ad996fef6ee50a29d8b8841.