The llvm patches have been posted as: <https://reviews.llvm.org/D45941>, < https://reviews.llvm.org/D45942>. On Fri, 20 Apr 2018 at 22:12, Greg Clayton <clayb...@gmail.com> wrote:
> > On Apr 20, 2018, at 12:17 PM, Pavel Labath <lab...@google.com> wrote: > > > > On Fri, 20 Apr 2018 at 17:14, Greg Clayton <clayb...@gmail.com> wrote: > >>> On Apr 20, 2018, at 1:08 AM, Pavel Labath <lab...@google.com> wrote: > >>> > >>> > >>> So, I can see the case for both, and I don't really have a clear > >>> preference. All I would say is, whichever way we choose, we should make > > it > >>> very explicit so that the users of FileSpec know what to expect. > > > >> I would say that without a directory it is a wildcard match on base name > > alone, and with one, the partial directories must match if the path is > > relative, and the full directory must match if absolute. I will submit a > > patch that keeps leading "./" and "../" during normalization and we will > > see what people think. > I have that as part of my current patch, so don't worry about that. > > > > Ok, what about multiple leading "./" components? Would it make sense to > > collapse those to a single one ("././././foo.cpp" -> "./foo.cpp")? > yes! > > > > > >>> > >>> On Thu, 19 Apr 2018 at 19:37, Zachary Turner via lldb-dev < > >>> lldb-dev@lists.llvm.org> wrote: > >>>> I think I might have tried to replace some of the low level functions > > in > >>> FileSpec with the LLVM equivalents and gotten a few test failures, but I > >>> didn't have time to investigate. It would be a worthwhile experiment > > for > >>> someone to try again if they have some cycles. > > > >> I took a look at the llvm file stuff and it has llvm::sys::fs::real_path > > which always resolves symlinks _and_ normalizes the path. Would be nice to > > break it out into two parts by adding llvm::sys::fs::normalize_path and > > have llvm::sys::fs::real_path call it. > > > >>> I can try to take a look at it. The way I remember it, I just copied > > these > >>> functions from llvm and replaced all #ifdefs with runtime checks, which > > is > >>> pretty much what you later did in llvm proper. Unless there has been > > some > >>> significant divergence since then, it shouldn't be hard to reconcile > > these. > > > > So, I tried playing around with unifying the two implementations today. I > > didn't touch the normalization code, I just wanted to try to replace path > > parsing functions with the llvm ones. > > > > In theory, it should be as simple as replacing our parsing code in > > FileSpec::SetFile with calls to llvm::sys::path::filename and > > ...::parent_path (to set m_filename and m_directory). > > > > It turned out this was not as simple, and the reason is that the llvm path > > api's aren't completely self-consistent either. For example, for "//", the > > filename+parent_path functions will decompose it into "." and "", while the > > path iterator api will just report a single component ("//"). > > > > After a couple of hours of fiddling with +/- ones, I think I have came up > > with a working and consistent implementation, but I haven't managed to > > finish polishing it today. I'll try to upload the llvm part of the patch on > > Monday. (I'll refrain from touching the lldb code for now, to avoid > > interfering with your patch). > Sounds good. Feel free to replace my changes with LLVM stuff as long as our test suite passes as I am adding a bunch of tests to cover all the cases. > Greg _______________________________________________ lldb-dev mailing list lldb-dev@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-dev