[libc-commits] [PATCH] D73530: [libc] Add a library of standalone C++ utilities.

[Alex Brachet via Phabricator via libc-commits]

abrachet accepted this revision.
abrachet added inline comments.
This revision is now accepted and ready to land.


================
Comment at: libc/utils/CPP/Array.h:14-15
+
+template <class T, size_t N> class Array {
+public:
+  T Data[N];
----------------
We could make this a struct?


================
Comment at: libc/utils/CPP/ArrayRef.h:37
+  // Construct an ArrayRef from a single element.
+  ArrayRef(const T &OneElt) : Data(&OneElt), Length(1) {}
+
----------------
Do we want this to be `explicit`? I think `ArrayRef<T> arr = someT;` is a little surprising. 


================
Comment at: libc/utils/CPP/ArrayRef.h:74-78
+  MutableArrayRef(const T *data, size_t length) : ArrayRef<T>(data, length) {}
+
+  // Construct an ArrayRef from a range.
+  MutableArrayRef(const T *begin, const T *end) : ArrayRef<T>(begin, end) {}
+
----------------
These are constructed from pointers to `const T` so arguably these constructors should not be allowed.


================
Comment at: libc/utils/CPP/ArrayRef.h:86
+  iterator begin() const { return data(); }
+  iterator end() const { return data() + this->size(); }
+
----------------
Unnecessary `this->`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73530