[libc-commits] [PATCH] D100732: [libc] Enhance ArrayRef + unittests

[Siva Chandra via Phabricator via libc-commits]

sivachandra accepted this revision.
sivachandra added a comment.
This revision is now accepted and ready to land.

Overall, a very nice change. Thanks for doing it. The clang-tidy warnings in the `equals` method are legitimate and should be fixed before submitting.



================
Comment at: libc/utils/CPP/ArrayRef.h:90
+  // equals - Check for element-wise equality.
+  bool equals(ArrayRefBase<QualifiedT> RHS) const {
+    if (Length != RHS.Length)
----------------
Calling it `equals` is a bit misleading when we are comparing references. Unless this name is inspired from other places in llvm-project, may be name it `equalData` or something like that?


================
Comment at: libc/utils/CPP/ArrayRef.h:113
+                "ArrayRef must have a non-const, non-volatile value_type");
+  using Impl = internal::ArrayRefBase<const T>;
+  using Impl::Impl;
----------------
Since this is a struct, may be make the using statements private?


================
Comment at: libc/utils/CPP/ArrayRef.h:125
+      "MutableArrayRef must have a non-const, non-volatile value_type");
+  using Impl = internal::ArrayRefBase<T>;
+  using Impl::Impl;
----------------
Same here?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100732