[Lldb-commits] [PATCH] D46783: FileSpec objects that resolve to "." should have "." in m_filename and m_directory empty.

2018-05-17 Thread Greg Clayton via Phabricator via lldb-commits
clayborg added a comment. Fixed with: svn commit unittests/Utility/FileSpecTest.cpp Sendingunittests/Utility/FileSpecTest.cpp Transmitting file data .done Committing transaction... Committed revision 332633. https://reviews.llvm.org/D46783

[Lldb-commits] [PATCH] D46783: FileSpec objects that resolve to "." should have "." in m_filename and m_directory empty.

2018-05-17 Thread Greg Clayton via Phabricator via lldb-commits
clayborg added a subscriber: labath. clayborg added a comment. Fixed with svn commit unittests/Utility/FileSpecTest.cpp Sendingunittests/Utility/FileSpecTest.cpp Transmitting file data .done Committing transaction... Committed revision 332633. https://reviews.llvm.org/D46783

Re: [Lldb-commits] [PATCH] D46783: FileSpec objects that resolve to "." should have "." in m_filename and m_directory empty.

2018-05-17 Thread Greg Clayton via lldb-commits
Fixed with svn commit unittests/Utility/FileSpecTest.cpp Sendingunittests/Utility/FileSpecTest.cpp Transmitting file data .done Committing transaction... Committed revision 332633. > On May 17, 2018, at 10:18 AM, Pavel Labath via Phabricator > wrote: > >

[Lldb-commits] [PATCH] D46783: FileSpec objects that resolve to "." should have "." in m_filename and m_directory empty.

2018-05-17 Thread Pavel Labath via Phabricator via lldb-commits
labath added a comment. http://lab.llvm.org:8011/builders/lldb-x86_64-ubuntu-14.04-cmake/builds/23447 https://reviews.llvm.org/D46783 ___ lldb-commits mailing list lldb-commits@lists.llvm.org

[Lldb-commits] [PATCH] D46783: FileSpec objects that resolve to "." should have "." in m_filename and m_directory empty.

2018-05-17 Thread Pavel Labath via Phabricator via lldb-commits
labath added a comment. Sorry, I was wrong about the cause, but the bots are red nonetheless. My bet is it's the last `{"", "."},` test, which is not working because of an early return in SetFile. TBH, I am not sure we would want that to work anyway, as we too treat an empty filespec specially

[Lldb-commits] [PATCH] D46783: FileSpec objects that resolve to "." should have "." in m_filename and m_directory empty.

2018-05-17 Thread Greg Clayton via Phabricator via lldb-commits
clayborg added a comment. I did run unit tests and they all passed here? https://reviews.llvm.org/D46783 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Re: [Lldb-commits] [PATCH] D46783: FileSpec objects that resolve to "." should have "." in m_filename and m_directory empty.

2018-05-17 Thread Greg Clayton via lldb-commits
I just updated the differential with the actual patch. If you tried to apply the old one, then try it again with the latest diff I just uploaded > On May 17, 2018, at 10:09 AM, Pavel Labath wrote: > > This has broken the unit tests. Looks like a bad merge that did not take

[Lldb-commits] [PATCH] D46783: FileSpec objects that resolve to "." should have "." in m_filename and m_directory empty.

2018-05-17 Thread Greg Clayton via Phabricator via lldb-commits
clayborg updated this revision to Diff 147345. clayborg added a comment. Updated to what was committed. https://reviews.llvm.org/D46783 Files: source/Utility/FileSpec.cpp unittests/Utility/FileSpecTest.cpp Index: unittests/Utility/FileSpecTest.cpp

Re: [Lldb-commits] [PATCH] D46783: FileSpec objects that resolve to "." should have "." in m_filename and m_directory empty.

2018-05-17 Thread Greg Clayton via lldb-commits
The patch in the differential is off, this is the fixed version of this patch? > On May 17, 2018, at 10:09 AM, Pavel Labath wrote: > > This has broken the unit tests. Looks like a bad merge that did not take > into account the refactoring in r332088. > On Thu, 17 May 2018 at

[Lldb-commits] [PATCH] D46783: FileSpec objects that resolve to "." should have "." in m_filename and m_directory empty.

2018-05-17 Thread Pavel Labath via Phabricator via lldb-commits
labath added subscribers: clayborg, davide, jingham, zturner. labath added a comment. This has broken the unit tests. Looks like a bad merge that did not take into account the refactoring in r332088. Repository: rL LLVM https://reviews.llvm.org/D46783

Re: [Lldb-commits] [PATCH] D46783: FileSpec objects that resolve to "." should have "." in m_filename and m_directory empty.

2018-05-17 Thread Pavel Labath via lldb-commits
This has broken the unit tests. Looks like a bad merge that did not take into account the refactoring in r332088. On Thu, 17 May 2018 at 17:16, Phabricator via Phabricator < revi...@reviews.llvm.org> wrote: > This revision was automatically updated to reflect the committed changes. > Closed by

[Lldb-commits] [PATCH] D46783: FileSpec objects that resolve to "." should have "." in m_filename and m_directory empty.

2018-05-17 Thread Phabricator via Phabricator via lldb-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL332618: FileSpec objects that resolve to . should have . in m_filename and… (authored by gclayton, committed by ). Herald added a subscriber: llvm-commits. Changed prior to commit:

[Lldb-commits] [PATCH] D46783: FileSpec objects that resolve to "." should have "." in m_filename and m_directory empty.

2018-05-17 Thread Pavel Labath via Phabricator via lldb-commits
labath accepted this revision. labath added a comment. This revision is now accepted and ready to land. It would still be interesting to align remove_dots with the "standard practice", but it looks like that would end up being a big project. I think we can live with tweak on our side in this

[Lldb-commits] [PATCH] D46783: FileSpec objects that resolve to "." should have "." in m_filename and m_directory empty.

2018-05-16 Thread Greg Clayton via Phabricator via lldb-commits
clayborg reclaimed this revision. clayborg added a comment. Reviving this patch so I can get the changes into LLDB. Clang is expecting an empty path in many locations and I don't feel comfortable changing remove_dots in clang after trying it. So I would like to use this patch to fix things in

[Lldb-commits] [PATCH] D46783: FileSpec objects that resolve to "." should have "." in m_filename and m_directory empty.

2018-05-16 Thread Greg Clayton via Phabricator via lldb-commits
clayborg abandoned this revision. clayborg added a comment. Fixed LLDB test botes with: Sendingunittests/Utility/FileSpecTest.cpp Transmitting file data .done Committing transaction... Committed revision 332511. https://reviews.llvm.org/D46783

[Lldb-commits] [PATCH] D46783: FileSpec objects that resolve to "." should have "." in m_filename and m_directory empty.

2018-05-15 Thread Pavel Labath via Phabricator via lldb-commits
labath added a comment. Thank you. https://reviews.llvm.org/D46783 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D46783: FileSpec objects that resolve to "." should have "." in m_filename and m_directory empty.

2018-05-15 Thread Greg Clayton via Phabricator via lldb-commits
clayborg added a comment. Submitted the LLVM patch: https://reviews.llvm.org/D46887 https://reviews.llvm.org/D46783 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D46783: FileSpec objects that resolve to "." should have "." in m_filename and m_directory empty.

2018-05-15 Thread Pavel Labath via Phabricator via lldb-commits
labath added a comment. If those are the only tests that fail, then I'd still go for it, as I still firmly believe this is the correct behavior for such a function. Such a low level test does not have to mean that someone really cares about this, it could be more of a case of documenting

[Lldb-commits] [PATCH] D46783: FileSpec objects that resolve to "." should have "." in m_filename and m_directory empty.

2018-05-15 Thread Greg Clayton via Phabricator via lldb-commits
clayborg added a comment. So I made a fix to LLVM and there are tests that are testing for the empty string: [ RUN ] Support.RemoveDots /Users/gclayton/Documents/src/llvm/clean/llvm/unittests/Support/Path.cpp:1149: Failure Expected: "" To be equal to: remove_dots(".\\", false,

[Lldb-commits] [PATCH] D46783: FileSpec objects that resolve to "." should have "." in m_filename and m_directory empty.

2018-05-15 Thread Pavel Labath via Phabricator via lldb-commits
labath added a comment. (Also, the function already does not remove leading `..` components, so there is precedent already for not removing stuff when it causes the path to become not equivalent.) https://reviews.llvm.org/D46783 ___ lldb-commits

[Lldb-commits] [PATCH] D46783: FileSpec objects that resolve to "." should have "." in m_filename and m_directory empty.

2018-05-15 Thread Pavel Labath via Phabricator via lldb-commits
labath added a comment. In https://reviews.llvm.org/D46783#1099561, @clayborg wrote: > So the function in llvm is called llvm::sys::path::remove_dots(...) and it is > removing the dots. Not sure it is correct to be changing a function that says > "remove_dots" to not remove dots and actually

[Lldb-commits] [PATCH] D46783: FileSpec objects that resolve to "." should have "." in m_filename and m_directory empty.

2018-05-15 Thread Greg Clayton via Phabricator via lldb-commits
clayborg added a comment. In https://reviews.llvm.org/D46783#1097549, @labath wrote: > Preserving the dot if it is the only path component is perfectly reasonable > -- "" is not a valid path and we shouldn't convert a valid path into an > invalid one. However, I think this should be done on

[Lldb-commits] [PATCH] D46783: FileSpec objects that resolve to "." should have "." in m_filename and m_directory empty.

2018-05-15 Thread Greg Clayton via Phabricator via lldb-commits
clayborg added a comment. So the function in llvm is called llvm::sys::path::remove_dots(...) and it is removing the dots. Not sure it is correct to be changing a function that says "remove_dots" to not remove dots and actually return something with a . in it... Seems like this should be taken

[Lldb-commits] [PATCH] D46783: FileSpec objects that resolve to "." should have "." in m_filename and m_directory empty.

2018-05-15 Thread Pavel Labath via Phabricator via lldb-commits
labath added a comment. Adding a test for that in lldb is fine. If we're relying on some behavior we want to make sure it doesn't change without us noticing. However, I don't think we should make any special allowments for people using lldb with older versions of llvm. It sends the wrong

[Lldb-commits] [PATCH] D46783: FileSpec objects that resolve to "." should have "." in m_filename and m_directory empty.

2018-05-14 Thread Greg Clayton via Phabricator via lldb-commits
clayborg added a comment. I agree, but we should still guard against it and test in in LLDB to ensure this doesn't regress. I'll be happy to make a LLVM patch. Many people have branches that link against older LLVMs and the check is cheap. https://reviews.llvm.org/D46783

[Lldb-commits] [PATCH] D46783: FileSpec objects that resolve to "." should have "." in m_filename and m_directory empty.

2018-05-14 Thread Pavel Labath via Phabricator via lldb-commits
labath added a comment. Preserving the dot if it is the only path component is perfectly reasonable -- "" is not a valid path and we shouldn't convert a valid path into an invalid one. However, I think this should be done on the llvm side, in the `remote_dots` function and not as a workaround

[Lldb-commits] [PATCH] D46783: FileSpec objects that resolve to "." should have "." in m_filename and m_directory empty.

2018-05-11 Thread Greg Clayton via Phabricator via lldb-commits
clayborg added a comment. This patch is needed for some PathMappingList fixes I have coming up. https://reviews.llvm.org/D46783 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D46783: FileSpec objects that resolve to "." should have "." in m_filename and m_directory empty.

2018-05-11 Thread Greg Clayton via Phabricator via lldb-commits
clayborg created this revision. clayborg added reviewers: labath, jingham, davide, zturner. After switching to LLVM normalization, if we init FileSpec with "." we would end up with m_directory being NULL and m_filename being "". This patch fixes this by allowing the path to be normalized and if