[PATCH] D51847: Print correctly dependency paths on Windows

2018-09-13 Thread Dávid Bolvanský via Phabricator via cfe-commits
This revision was not accepted when it landed; it landed in state "Needs Review". This revision was automatically updated to reflect the committed changes. Closed by commit rC342139: Print correctly dependency paths on Windows (authored by xbolva00, committed by ). Repository: rC Clang https:

[PATCH] D51847: Print correctly dependency paths on Windows

2018-09-12 Thread Dávid Bolvanský via Phabricator via cfe-commits
xbolva00 updated this revision to Diff 165204. xbolva00 added a comment. Adjusted regexes in two more tests https://reviews.llvm.org/D51847 Files: lib/Frontend/DependencyFile.cpp test/Frontend/dependency-gen-escaping.c test/Frontend/dependency-gen.c test/Modules/relative-dep-gen.cpp In

[PATCH] D51847: Print correctly dependency paths on Windows

2018-09-12 Thread Mailing List "cfe-commits" via Phabricator via cfe-commits
cfe-commits added a comment. Lgtm - F7184192: msg-6991-166.txt https://reviews.llvm.org/D51847 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D51847: Print correctly dependency paths on Windows

2018-09-12 Thread Zachary Turner via cfe-commits
Lgtm On Wed, Sep 12, 2018 at 3:23 AM Dávid Bolvanský via Phabricator < revi...@reviews.llvm.org> wrote: > xbolva00 updated this revision to Diff 165052. > xbolva00 added a comment. > > - Fixed failing test > > > https://reviews.llvm.org/D51847 > > Files: > lib/Frontend/DependencyFile.cpp > tes

[PATCH] D51847: Print correctly dependency paths on Windows

2018-09-12 Thread Dávid Bolvanský via Phabricator via cfe-commits
xbolva00 updated this revision to Diff 165052. xbolva00 added a comment. - Fixed failing test https://reviews.llvm.org/D51847 Files: lib/Frontend/DependencyFile.cpp test/Frontend/dependency-gen-escaping.c Index: test/Frontend/dependency-gen-escaping.c =

[PATCH] D51847: Print correctly dependency paths on Windows

2018-09-11 Thread Dávid Bolvanský via Phabricator via cfe-commits
xbolva00 updated this revision to Diff 165012. xbolva00 added a comment. Convert paths to platform native paths https://reviews.llvm.org/D51847 Files: lib/Frontend/DependencyFile.cpp Index: lib/Frontend/DependencyFile.cpp === -

[PATCH] D51847: Print correctly dependency paths on Windows

2018-09-11 Thread Zachary Turner via Phabricator via cfe-commits
zturner added a comment. clang-cl in general tries to do the thing that makes native Windows developers the most comfortable, while gcc in general tries to do the thing that makes Unix developers most comfortable. So I think we should probably use \ here. If anyone complains we can revisit.

[PATCH] D51847: Print correctly dependency paths on Windows

2018-09-11 Thread Dávid Bolvanský via Phabricator via cfe-commits
xbolva00 added a comment. In https://reviews.llvm.org/D51847#1230766, @zturner wrote: > Alright I see it. The reason I'm asking is because after your change you're > now printing `main.o: main.c ../include/lib/test.h`. But I wonder if you > should instead be printing `main.o: main.c ..\includ

[PATCH] D51847: Print correctly dependency paths on Windows

2018-09-11 Thread Zachary Turner via Phabricator via cfe-commits
zturner added a comment. Alright I see it. The reason I'm asking is because after your change you're now printing `main.o: main.c ../include/lib/test.h`. But I wonder if you should instead be printing `main.o: main.c ..\include\lib\test.h`. It seems like paths that we show to the user should

[PATCH] D51847: Print correctly dependency paths on Windows

2018-09-11 Thread Dávid Bolvanský via Phabricator via cfe-commits
xbolva00 added a comment. Please see PR38877. https://reviews.llvm.org/D51847 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D51847: Print correctly dependency paths on Windows

2018-09-11 Thread Zachary Turner via Phabricator via cfe-commits
zturner added a comment. I mean in practice. What command do i run to hit this and what will the output look like? Can you paste some terminal output showing the command and output? https://reviews.llvm.org/D51847 ___ cfe-commits mailing list cfe-co

Re: [PATCH] D51847: Print correctly dependency paths on Windows

2018-09-11 Thread Zachary Turner via cfe-commits
I mean in practice. What command do i run to hit this and what will the output look like? Can you paste some terminal output showing the command and output? On Tue, Sep 11, 2018 at 12:55 AM Dávid Bolvanský via Phabricator < revi...@reviews.llvm.org> wrote: > xbolva00 added a comment. > > In https:

[PATCH] D51847: Print correctly dependency paths on Windows

2018-09-11 Thread Dávid Bolvanský via Phabricator via cfe-commits
xbolva00 added a comment. In https://reviews.llvm.org/D51847#1228927, @zturner wrote: > What prints this? How do you exercise this codepath? DFGImpl::OutputDependencyFile() -> for (StringRef File : Files) https://reviews.llvm.org/D51847 ___ cfe-c

[PATCH] D51847: Print correctly dependency paths on Windows

2018-09-10 Thread Zachary Turner via Phabricator via cfe-commits
zturner added a subscriber: xbolva00. zturner added a comment. What prints this? How do you exercise this codepath? https://reviews.llvm.org/D51847 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinf