[PATCH] D47090: Implement C++17 <memory_resource>.

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

Quuxplusone added inline comments.


================
Comment at: include/__memory_resource_base:196
+              typename __uses_alloc_ctor<
+                  _T1, polymorphic_allocator&, _Args1...
+              >::type()
----------------
>> (B) It's got different semantics around uses-allocator construction because of https://wg21.link/lwg2969
> Issue resolutions should probably be applied to the experimental versions as well.

Okay, I can roll with that. I'll create a new patch that modifies `<experimental/memory_resource>`'s uses-allocator parts per LWG2969 (and my followup DR), without any other diffs.

Should `<experimental/memory_resource>` continue caring about `std::experimental::erased_type`, which was in LFTS and LFTSv2 but did not make it into C++17?  My kneejerk reaction is "yes".  (And at the end of this process, when we copy `<experimental/memory_resource>` into `<memory_resource>`, should `<memory_resource>` care about `erased_type`? My kneejerk reaction is "no".)


================
Comment at: src/memory_resource.cpp:62
+
+namespace {
+
----------------
EricWF wrote:
> We certainly don't want a different definition of the global resources in each TU. See below.
@EricWF: I think all of your comments in this file are the result of misreading "src/memory_resource.cpp" as "include/memory_resource".  Or else I *totally* don't understand the current directory organization and you're going to have to instruct me. :)



Repository:
  rCXX libc++

https://reviews.llvm.org/D47090