Skip to content

Conversation

CaseyCarter
Copy link
Contributor

...by LWG-3170. We improperly reused the "allocator features deprecated in C++17" deprecation warning macro instead of devising a new macro name, and therefore incorrectly "promoted" is_always_equal to "removed" when the other such marked features were removed for C++20.

Fixes #2422

...by LWG-3170. We improperly reused the "allocator features deprecated in C++17" deprecation warning macro instead of devising a new macro name, and therefore incorrectly "promoted" `is_always_equal` to "removed" when the other such marked features were removed for C++20.

Fixes microsoft#2422
@CaseyCarter CaseyCarter added the bug Something isn't working label Dec 17, 2021
@CaseyCarter CaseyCarter requested a review from a team as a code owner December 17, 2021 04:06
@StephanTLavavej
Copy link
Member

Looks good to me. It's somewhat unusual that this was deprecated by an LWG issue resolution, which we generally treat as retroactive back to the beginning of time (or when a feature appeared). Given when this was voted in (and C++20's, ahem, "extended" finalization period), I think that your approach makes sense. Extending this further back to C++17 would be too aggressive, I think - and delaying it to C++23 would serve no real purpose.

https://en.cppreference.com/w/cpp/memory/allocator describes the deprecation as happening in C++23 but I'm okay with diverging from that view here.

@StephanTLavavej StephanTLavavej changed the title allocator::is_always_equal is deprecated, not removed, in C++20 allocator::is_always_equal is deprecated, not removed, in C++20 Dec 18, 2021
@frederick-vs-ja
Copy link
Contributor

Perhaps extending the deprecation back to C++14 mode (i.e. applying it to all modes) is also OK?
A funny fact is that although is_always_equal is a C++17 component, all known implementations extends it to earlier modes (libc++: C++98/03, libstdc++: C++11, MSVC STL: C++14), as if WG21-N4258 were a defect report (against C++11 IMO).

@StephanTLavavej
Copy link
Member

@frederick-vs-ja Because C++14 mode is the default, emitting deprecation warnings in that mode is extremely aggressive - thus we generally avoid doing so. (An exception is tr1, which is so ancient that we've deprecated in C++14 mode and removed by default in C++17 mode.) The whole purpose of Standard modes is to allow users with legacy codebases to continue compiling with minimal changes as they upgrade toolsets. Because deprecation warnings merely indicate that code can/should be modernized but is not inherently wrong, emitting such warnings in the highly compatible C++14 mode doesn't align with its intent - whereas if a user has opted in to C++17 or above then they've indicated that conforming to modern-ish Standards is a priority for them, so emitting deprecation warnings is increasingly valuable there.

@StephanTLavavej
Copy link
Member

I'm mirroring this to the MSVC-internal repo - please notify me if any further changes are pushed.

@StephanTLavavej StephanTLavavej merged commit 3355484 into microsoft:main Jan 20, 2022
@StephanTLavavej
Copy link
Member

Thanks for fixing this overly-eager removal! 🛠️ 🎉 😸

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

<xmemory>: allocator::is_always_equal is incorrectly removed together with members removed in C++20
5 participants