mgorny marked an inline comment as done.
mgorny added inline comments.
================
Comment at: llvm/include/llvm/ADT/StringExtras.h:544
/// of the iterators.
-class Split {
- StringRef Str;
- std::string SeparatorStr;
-
-public:
- Split(StringRef NewStr, StringRef Separator)
- : Str(NewStr), SeparatorStr(Separator) {}
- Split(StringRef NewStr, char Separator)
- : Str(NewStr), SeparatorStr(1, Separator) {}
-
- SplittingIterator begin() { return SplittingIterator(Str, SeparatorStr); }
-
- SplittingIterator end() { return SplittingIterator("", SeparatorStr); }
-};
+inline iterator_range<SplittingIterator> Split(StringRef Str, StringRef
Separator) {
+ return {SplittingIterator(Str, Separator),
----------------
labath wrote:
> this should also be a lower-case `split` now that it's a function.
>
> (Believe it or not, this is the reason I was first drawn to this -- it's
> uncommon to see a class used like this because, even in the cases where you
> do have a separate class, you usually create a function wrapper for it. The
> original reason for this is pragmatic (use of template argument deduction),
> but the practice is so ubiquitous that a deviation from it stands out.)
Yeah, that makes sense. In fact, I originally named the class lowercase for
this exact reason.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D110535/new/
https://reviews.llvm.org/D110535
_______________________________________________
lldb-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
