labath added inline comments.
================
Comment at: source/Utility/FileSpec.cpp:406
+ m_directory.Clear();
+ }
}
----------------
clayborg wrote:
> > they resolve to the same file in a virtual filesystem, where all referenced
> > path components exist and are directories
>
> Normalizing happens after resolving and if we don't resolve a path, we have
> no way to know what is a directory and what isn't. We will be setting
> breakpoints for remote targets quite a bit in LLDB and ww can't assume or
> stat anything in a path. So I would say FileSpec's are equivalent if the
> relative paths match all components.
>
> > Now the (c) part would imply that foo/.. should normalize to ./. and not .,
> > which is a bit odd, but it is consistent with our stated intention of
> > preserving directory information. If we do not want to have ./. here, then
> > we need to come up with a different definition of what it means to be
> > "normalized".
>
> I guess we could make the m_directory contain "." and m_filename contain
> nothing for the "./." case. It doesn 't make sense to have "." in both
> places.
>> they resolve to the same file in a virtual filesystem, where all referenced
>> path components exist and are directories
> Normalizing happens after resolving and if we don't resolve a path, we have
> no way to know what is a directory and what isn't. We will be setting
> breakpoints for remote targets quite a bit in LLDB and ww can't assume or
> stat anything in a path.
Yes, I am aware of that. I am not saying this is how we should actually
implement the normalization algorithm. I am trying define what a
"normalization" is in the first place, so that we can then judge whether a
particular normalization algorithm is good or not. I think defining
normalization in terms of an actual filesystem makes sense, since at the end of
the day, our algorithm should somehow approximate what happens in real file
systems. I am not saying the algorithm should be doing any stats, but for the
verification (either in our heads or in the tests) we can use certainly use
stats or actual file systems.
> So I would say FileSpec's are equivalent if the relative paths match all
> components.
This is too vague to be useful. I have no idea how I would apply this
definition to determine if e.g. "/foo/../bar.txt" and "./bar.txt" are
equivalent. And you didn't say anything about how to derive the normal form for
a `FileSpec`.
>> Now the (c) part would imply that foo/.. should normalize to ./. and not .,
>> which is a bit odd, but it is consistent with our stated intention of
>> preserving directory information. If we do not want to have ./. here, then
>> we need to come up with a different definition of what it means to be
>> "normalized".
> I guess we could make the m_directory contain "." and m_filename contain
> nothing for the "./." case. It doesn 't make sense to have "." in both places.
I don't think that is very useful, as then this would be the only special case
where normalization would produce a FileSpec without a filename component.
================
Comment at: source/Utility/FileSpec.cpp:857
+ !IsPathSeparator(components[i].back(), syntax) &&
+ (result.empty() || !IsPathSeparator(result.back(), syntax)))
result += GetPreferredPathSeparator(syntax);
----------------
clayborg wrote:
> labath wrote:
> > Why is this necessary? `result` can't be empty because we have just
> > appended something to it (and we've checked that `components[i]` is
> > non-empty).
> Because if you don't check this joining {"/', "foo.txt"} will result in
> "//foo.txt" which is wrong,.
Yes, but didn't the original condition guard against that already?
We know that `components[i]` is non-empty, and we have just appended it to
`result` two lines above. So, unless I am missing something, `result.back()`
should be the same as `components[i].back()` and this additional check does not
buy us anything.
https://reviews.llvm.org/D45977
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
