clayborg added inline comments.
================
Comment at: source/Utility/FileSpec.cpp:107
+ for (auto i = path.find('/'); i != llvm::StringRef::npos;
+ i = path.find('/', i + 1)) {
+ const auto nextChar = safeCharAtIndex(path, i+1);
----------------
aprantl wrote:
> clayborg wrote:
> > I am fine switching to using the llvm functions for removing, this is only
> > detecting if any normalization needs to happen. If there is an equivalent
> > LLVM function that will only run through the string one time to detect all
> > needs for normalization (no multiple passes looking for "..", then for "."
> > etc like we used to have), I would be happy to use it, but there doesn't
> > seem to be. We are trying to avoid having to create a "SmallVectorImpl<
> > char >" for all paths if they don't need fixing here. This function is just
> > iterating through an llvm::StringRef just to see if normalization needs to
> > happen. If it does, then we make a SmallVectorImpl< char > and we fix the
> > path. We can easily use llvm functions for the fixing part.
> I see, you want to avoid copying the string when it isn't necessary to do so.
> IMHO the best way to do this is to add a `bool hasDots(const Twine &)`
> function to llvm::sys::path and add a `bool remove_dot_dot` parameter to
> `removeDots()`. Since you have already written the unittests, adding the
> function to LLVM shouldn't be any extra work (I'll help reviewing), and we'll
> have 100 LoC less to maintain.
>
> This way we can avoid having two functions that perform path normalization
> that superficially look like they might do the same thing, but a year from
> now nobody can really tell whether they behave exactly the same, and whether
> a bug found in one implementation should also be fixed in the other one,
> etc... .
I want to know if there are redundant slashes as well. I want something like
"bool llvm::sys::has(const Twine &, bool dots, bool dot_dots, bool
redundant_slashes)". But I don't see that getting accepted? I just want a "bool
llvm:sys::path::contains_redundant_stuff(const Twine &)". Not sure on the name
though. needs_normalization? can_be_shortened? Everything in LLVM right now
assumes that dots, dot dots, slashes and realpath stuff happen in the same
function. Not sure how to break that up without ruining the performance of the
one and only loop that should be happening. I like the current approach since
it doesn't require chopping up the string into an array and iterating through
the array.
https://reviews.llvm.org/D45977
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
