Deprecate Volatile

[miscco]

Description

This fixes #558 by removing the appropriate parts from the STL.

I verified those tests that do not require libc++ machinery. For the sake of my sanity someone with acces to a test runner could tell me what I have to change for the tests, as I am not sure whether deprecation is an actual problem and / or whether we would actually want to suppress the warning for the tests.

Checklist

Be sure you've read README.md and understand the scope of this repo.

If you're unsure about a box, leave it unchecked. A maintainer will help you.

• Identifiers in product code changes are properly _Ugly as per
  https://eel.is/c++draft/lex.name#3.1 or there are no product code changes.
• The STL builds successfully and all tests have passed (must be manually
  verified by an STL maintainer before automated testing is enabled on GitHub,
  leave this unchecked for initial submission).
• These changes introduce no known ABI breaks (adding members, renaming
  members, adding virtual functions, changing whether a type is an aggregate
  or trivially copyable, etc.).
• These changes were written from scratch using only this repository,
  the C++ Working Draft (including any cited standards), other WG21 papers
  (excluding reference implementations outside of proposed standard wording),
  and LWG issues as reference material. If they were derived from a project
  that's already listed in NOTICE.txt, that's fine, but please mention it.
  If they were derived from any other project (including Boost and libc++,
  which are not yet listed in NOTICE.txt), you must mention it here,
  so we can determine whether the license is compatible and what else needs
  to be done.

[miscco]

Me: Lets delete some code 👍

Also me: Adds 274 lines of #ifdef magic

[BillyONeal]

For the sake of my sanity someone with acces to a test runner

Curtis is very very close to landing #520 so that will soon be you :)

[BillyONeal]

@CaseyCarter Since this is a C++20 only feature can we use a requires clause to turn these off or are there IFNDR concerns since in 20 mode it will always be disabled?

[StephanTLavavej]

Regarding concepts usage, please see #602.

[miscco]

@BillyONeal I actually only went for enable_if_t because the requires clause seemed to have required more line changes and was IMHO less readable, e.g.

#if _HAS_CXX20
    template <class _Type = _Ty, enable_if_t<_Is_always_lock_free<_Type>, int> = 0>
#endif // _HAS_CXX20
    _Ty fetch_sub(const _Ty _Operand) volatile noexcept {
        return fetch_add(_Negate(_Operand));
    }

vs

    _Ty fetch_sub(const _Ty _Operand) volatile noexcept
#if _HAS_CXX20
    requires _Is_always_lock_free<_Type>
#endif // _HAS_CXX20
    {
        return fetch_add(_Negate(_Operand));
    }    

That said, I do not believe it is that bad after all

[BillyONeal]

@BillyONeal I actually only went for enable_if_t because the requires clause seemed to have required more line changes and was IMHO less readable

That's fair. I'm not arguing for doing that explicitly, only making sure that we aren't avoiding the feature just because we think it is unavailable.

[miscco]

Could somebody start a run of the test suite?

[StephanTLavavej]

/azp run

[azure-pipelines]

Azure Pipelines successfully started running 1 pipeline(s).

[StephanTLavavej]

We'll probably need to add deprecation suppressions to tr1 too (that's not harnessed in GitHub yet so we'll need to run the Microsoft-internal test harness and push changes to your branch).

[cbezault]

/azp run

[azure-pipelines]

Azure Pipelines successfully started running 1 pipeline(s).

[miscco]

@CaseyCarter I would put those two failing tests in the skip list or are there some other things you would want to try here?

[miscco]

Rebased on master and deactivated the two failing libcxx tests.

That said, I am unsure why test for volatile atomic<bool> and volatile atomic<int> fail. They should be always lock free

[CaseyCarter]

Note to other reviewers: Despite that WG21-P1831 doesn't mention these other non-member functions explicitly, they are specified as equivalent to calling the (now conditionally deprecated) member functions in [atomics.nonmembers]/1, so they should be deprecated.

[StephanTLavavej]

After pushing the partial specialization change, I noticed that the deprecation machinery in integral/floating/pointer is unnecessary - they are always lock-free even in our current ABI (as their size is <= 8 and they're powers of 2). @CaseyCarter agreed, so I removed those occurrences and added comments. However, I left all of the occurrences in the non-member functions like atomic_fetch_add.

[StephanTLavavej]

Thanks for yet another C++20 feature, @miscco! 😺😸