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);
----------------
clayborg wrote:
> 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.
Also not sure if the LLVM stuff will try to substitute in the current working
directory for "."?
https://reviews.llvm.org/D45977
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
