[libcxx-commits] [PATCH] D114761: [libc++][ranges] Implement [special.mem.concepts].

[Christopher Di Bella via Phabricator via libcxx-commits]

cjdb requested changes to this revision.
cjdb added a comment.

Very close to LGTM, thanks for working on this! I'd like to see the concepts in a descriptively named file before continuing.



================
Comment at: libcxx/include/__memory/ranges_uninitialized_algorithms.h:1
+// -*- C++ -*-
+//===----------------------------------------------------------------------===//
----------------
Would you mind renaming this file to `uninitialized_algo_concepts.h` or similar please?


================
Comment at: libcxx/include/module.modulemap:633
     module __memory {
-      module addressof                { private header "__memory/addressof.h"                }
-      module allocation_guard         { private header "__memory/allocation_guard.h"         }
-      module allocator                { private header "__memory/allocator.h"                }
-      module allocator_arg_t          { private header "__memory/allocator_arg_t.h"          }
-      module allocator_traits         { private header "__memory/allocator_traits.h"         }
-      module auto_ptr                 { private header "__memory/auto_ptr.h"                 }
-      module compressed_pair          { private header "__memory/compressed_pair.h"          }
-      module construct_at             { private header "__memory/construct_at.h"             }
-      module pointer_traits           { private header "__memory/pointer_traits.h"           }
-      module raw_storage_iterator     { private header "__memory/raw_storage_iterator.h"     }
-      module shared_ptr               { private header "__memory/shared_ptr.h"               }
-      module temporary_buffer         { private header "__memory/temporary_buffer.h"         }
-      module uninitialized_algorithms { private header "__memory/uninitialized_algorithms.h" }
-      module unique_ptr               { private header "__memory/unique_ptr.h"               }
-      module uses_allocator           { private header "__memory/uses_allocator.h"           }
+      module addressof                         { private header "__memory/addressof.h"                       }
+      module allocation_guard                  { private header "__memory/allocation_guard.h"                }
----------------
As a general thing, I didn't bother to align the closing braces when making the initial header splits, and don't care for this change. It's very brittle on the LHS, and I don't want to duplicate said brittleness.


================
Comment at: libcxx/test/std/iterators/iterator.requirements/iterator.concepts/iterator.concept.input/input_iterator.compile.pass.cpp:26
 
-  no_explicit_iter_concept() = default;
-
----------------
ldionne wrote:
> Quuxplusone wrote:
> > ldionne wrote:
> > > Why are we removing those here?
> > This file had much more refactoring in the first revision, and then I said "let's split out all the controversial refactoring into a new PR." This is just what was left over. IMO this file should be landed in its own separate commit, and that could be done right now today AFAIC.
> I think it would make sense to split this in a separate diff and explain why we're removing those. It's probably a one liner to explain it, but I'd like it on record.
Agreed. Please keep commits small and relevant.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114761