[libcxx-commits] [PATCH] D100318: [libc++] Split various components out of <memory>

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

Quuxplusone added a comment.

> I disagree with the way you calculate complexity. Yes, the graph has more nodes and edges. But each file now contains a set of logically-related things, and is much smaller. As a human, I spend time reading files with code in it, not looking at graphs.

Me too! ;) When I'm reading that code, though, I tend to stumble over two measures of complexity:
(1) How hard do I have to look to find the source file that actually contains the thing I'm working on? (Once the right file is open, I can jump to the right line quickly; but I don't use an IDE that can jump me to the right //file// quickly.)
(2) How tightly entangled is this source file with other source files? (How many other files do I need to read in order to fully understand the code in //this// file?)
These two measures of complexity map directly onto "number of nodes in the graph" and "number of edges in the graph."
I do think minimizing edges is generally more important than minimizing nodes (and of course it's //most// important to keep the number of cycles at 0). Minimizing the //diameter// of the graph (the number of levels of abstraction you have to dig through to work on the code) is also relevant, if maybe less important than the other measures listed here.



================
Comment at: libcxx/include/__memory/unique_ptr.h:16
+#include <__memory/allocator_traits.h> // __pointer
+#include <__memory/auto_ptr.h>
+#include <__memory/compressed_pair.h>
----------------
ldionne wrote:
> Quuxplusone wrote:
> > This is a very surprising edge, since ultimately we want to remove `auto_ptr` altogether. Can you try to remove it? (At the very least, comment why it's here.)
> After the move is done, I will go back and make this dependent on the standard version and whether `auto_ptr` is force-enabled. I didn't want to change any code as part of these commits.
Ah, I get it now — the culprit is literally the constructor `unique_ptr(auto_ptr<Y>&&)`, which is removed only in C++20. Okay (although I'd still prefer to see the code comment `// std::auto_ptr` for clarity).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100318