[libcxx-commits] [PATCH] D105905: [libcxx] [test] Fix experimental/memory.resource.adaptor.mem/db_deallocate on Windows

[Arthur O'Dwyer via Phabricator via libcxx-commits]

Quuxplusone added subscribers: joerg, Quuxplusone.
Quuxplusone requested changes to this revision.
Quuxplusone added a comment.
This revision now requires changes to proceed.

Looking at the git history of `<experimental/memory_resource>`, I can see that the calculation has always been

      static const size_t _MaxAlign = alignof(max_align_t);
  [...]
      size_t __max_size() const _NOEXCEPT {
          return numeric_limits<size_t>::max() - _MaxAlign;
      }

Obviously `__STDCPP_DEFAULT_NEW_ALIGNMENT__` has never been relevant to this behavior, and shouldn't have been mentioned in its tests.

This particular bogus test code was introduced by @joerg in D73245 <https://reviews.llvm.org/D73245>. It might be worth (either @joerg or @mstorsjo) auditing all other places where D73245 <https://reviews.llvm.org/D73245> changed test code, to see whether they're also referring to `__STDCPP_DEFAULT_NEW_ALIGNMENT__` in counterfactual ways. Alternatively, maybe `resource_adaptor::_MaxAlign` should be updated to actually care about `__STDCPP_DEFAULT_NEW_ALIGNMENT__`, in which case the test would become correct.

Requesting "changes" in the form of investigation/discussion. If the result of investigation ends up being "yeah, `_MaxAlign` should continue not referring to `__STDCPP_DEFAULT_NEW_ALIGNMENT__`, and this is the only affected test case," then I'd happily approve this PR in its current form, since it's just bringing the test suite into line with libc++'s actual behavior.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D105905/new/

https://reviews.llvm.org/D105905