================
Comment at: lib/Frontend/DependencyFile.cpp:298
@@ -295,3 +297,3 @@
for (unsigned i = 0, e = Filename.size(); i != e; ++i) {
- if (Filename[i] == ' ' || Filename[i] == '#')
+ if (Filename[i] == ' ' || Filename[i] == '#') {
OS << '\\';
----------------
probinson wrote:
> silvas wrote:
> > probinson wrote:
> > > silvas wrote:
> > > > Why `#` also? We are trying to be gcc-compatible. See
> > > > https://gcc.gnu.org/git/?p=gcc.git;a=blob;f=libcpp/mkdeps.c;h=78bbf09a0cc8c3ba0fe974b55e1ededf02c66665;hb=HEAD#l47
> > > No, we're trying to produce something that GNU Make can use. What gcc
> > > produces is broken by any definition.
> > >
> > > $ cat tspace.c
> > > #include "foo\ bar.h"
> > > #include "foo\#bar.h"
> > > int i;
> > > $ ls
> > > foo\ bar.h foo\#bar.h tspace.c
> > > $ gcc -c -MMD tspace.c
> > > $ cat tspace.d
> > > tspace.o: tspace.c foo\\\ bar.h foo\\#bar.h
> > > $ make -f tspace.d tspace.o
> > > make: *** No rule to make target `foo\', needed by `tspace.o'. Stop.
> > > $
> > >
> > Please file a bug with gcc.
> >
> > Also, curiously that .d file seems to work for me on windows under cygwin:
> >
> > ```
> > Sean@Sean-PC ~
> > $ cat >tspace.c
> > #include "foo\ bar.h"
> > #include "foo\#bar.h"
> > int i;
> >
> > Sean@Sean-PC ~
> > $ mkdir foo
> >
> > Sean@Sean-PC ~
> > $ cd foo
> >
> > Sean@Sean-PC ~/foo
> > $ mkdir ' bar.h'
> >
> > Sean@Sean-PC ~/foo
> > $ mkdir '#bar.h'
> >
> > Sean@Sean-PC ~/foo
> > $ cd ..
> >
> > Sean@Sean-PC ~
> > $ cat >tspace.d
> > tspace.o: tspace.c foo\\\ bar.h foo\\#bar.h
> >
> > Sean@Sean-PC ~
> > $ make -f tspace.d
> > cc -c -o tspace.o tspace.c
> > make: cc: Command not found
> > <builtin>: recipe for target 'tspace.o' failed
> > make: *** [tspace.o] Error 127
> > ```
> >
> > Also, clang's current output seems to work fine on cygwin. In what cases is
> > clang's output currently breaking? Is it just the case of filenames
> > containing backslashes followed by space/hash on unix?
> >
> > The set of strings that can be correctly escaped in make is inherently
> > limited, so we are inherently compromising on what file names can be
> > properly represented. Does it make sense to extend the set of properly
> > representable filenames to files containing backslashes in their name?
> >
> > E.g. include "foo\ bar.h". On unix we would be instructing make to look for
> > a file 'foo\ bar.h' in the current directory but on windows in my testing
> > above we would be instructing make to look for a file ' bar.h' in directory
> > 'foo'. Any project for which that would actually be correct behavior is
> > insane. I think that realistically, backslashes are only going to end up in
> > includes on windows where they will be intended as directory separators,
> > and will result in compilation errors on other platforms. I believe that
> > clang's current output actually works correctly in that case.
> >
> > We might be overthinking this. What case are we fixing? And does it make
> > sense to fix that case?
> Cygwin is not doing the right thing for me. Using Clang's depfile:
> {code}
> probinson@PROBINSON-T3610 ~
> $ /cygdrive/e/upstream/build/debug/bin/clang -c -MMD tspace.c
>
> probinson@PROBINSON-T3610 ~
> $ cat tspace.d
> tspace.o: tspace.c foo\\ bar.h foo\\#bar.h
>
> probinson@PROBINSON-T3610 ~
> $ touch 'foo\#bar.h'
>
> probinson@PROBINSON-T3610 ~
> $ make -f tspace.d tspace.o
> make: *** No rule to make target 'bar.h', needed by 'tspace.o'. Stop.
> {code}
> Escaping the space is no good unless you escape the preceding backslash.
>
> With gcc's depfile:
> {code}
> probinson@PROBINSON-T3610 ~
> $ cat tspace.d
> tspace.o: tspace.c foo\\\ bar.h foo\\#bar.h
>
> probinson@PROBINSON-T3610 ~/foo
> $ touch 'foo/#bar.h'
>
> probinson@PROBINSON-T3610 ~
> $ make -f tspace.d tspace.o
> make: 'tspace.o' is up to date.
> {code}
> I suspect it's looking at directory foo, and treating #bar.h as a comment.
>
> But if I add the extra backslash manually, it works correctly.
> {code}
> probinson@PROBINSON-T3610 ~
> $ cat tspace.d
> tspace.o: tspace.c foo\\\ bar.h foo\\\#bar.h
>
> probinson@PROBINSON-T3610 ~
> $ touch 'foo/#bar.h'
>
> probinson@PROBINSON-T3610 ~
> $ make -f tspace.d tspace.o
> cc -c -o tspace.o tspace.c
> {code}
>
> So, what am I fixing? If we're going to do anything special with space and
> hash, then we should do a special thing that causes them to work properly
> with a tool that knows how to interpret the special thing. The only tool I
> have handy that is willing to do a special thing with space and hash is GNU
> Make (on Linux or Windows). The special thing to do is backslash-quote those
> characters. But that doesn't work in GNU Make unless you also backslash-quote
> the preceding backslashes; and that characteristic is the same for quoting
> either space or hash.
>
> Therefore, what clang produces won't work at all, and what gcc produces will
> work only for space and not for hash. My fix makes Clang's output work for
> both space and hash. Provably.
>
> I freely admit--always have, I think--that this is a rather obscure corner
> case that just isn't going to happen in practice. But it's provably wrong,
> and my fix provably corrects it. This is a problem?
>
> Alternatively, we can just abandon any special processing for space and hash,
> and say Don't Do That. But leaving this thing in its current provably wrong
> state just leaves me professionally offended.
>
And why I want to fix this... aside from the professionally-offended part...
:-)
We are looking at modifying the PS4 toolchain's builder to move away from
double-quotes, to doing things the GNU-Make style way. But I don't want to
require our builder to correctly interpret broken depfiles; Clang should be
producing correctly formed depfiles, and our builder should be able to depend
on depfiles being correctly formed.
http://reviews.llvm.org/D9208
EMAIL PREFERENCES
http://reviews.llvm.org/settings/panel/emailpreferences/
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
