[PATCH] Fix dependency file escaping

Nico Weber thakis at chromium.org
Mon May 11 10:21:01 PDT 2015


I don't think ninja's current behavior is interesting. My point is more
that it should be possible to write one program that can parse .d files
produced by both gcc and clang. If gcc and clang agree on some "escaping",
that's possible, else it isn't. If this patch makes clang match gcc's
output for '\'s in path names that's cool. I'm skeptical if it's worth
diverging on '#' characters.

On Mon, May 11, 2015 at 8:19 AM, Robinson, Paul <
Paul_Robinson at playstation.sony.com> wrote:

>  Okay, I worked out how to persuade ninja to read a depfile.  And in fact
> ninja 1.3.4 on Ubuntu 14.04 will treat
>
>     tspace.o: tspace.c bar\\#foo.h
>
> and
>
>     tspace.o: tspace.c bar\\\#foo.h
>
> as both expressing a dependency on the file named "bar\#foo.h" so the
> clang patch works for ninja.
>
> --paulr
>
>
>
> *From:* cfe-commits-bounces at cs.uiuc.edu [mailto:
> cfe-commits-bounces at cs.uiuc.edu] *On Behalf Of *Robinson, Paul
> *Sent:* Monday, May 11, 2015 7:58 AM
> *To:* Sean Silva
> *Cc:* reviews+D9208+public+0ac91376ffbd3e34 at reviews.llvm.org;
> cfe-commits at cs.uiuc.edu
> *Subject:* RE: [PATCH] Fix dependency file escaping
>
>
>
> Is this right? GCC and the current patch coincide on the handling of `\ `.
> The only deviation is in the `#` case, no?
>
>
>
> Sorry, you are correct, GCC was getting "\ " right but "\#" wrong.  Clang
> was doing both wrong.
>
> So, for Nico's example, "\ " would be converted to "\\\ " by gcc, but not
> by Clang, and that's being fixed by the patch.
>
> But GCC would emit "\#" as "\\#" which GNU make would interpret as "\"
> followed by a comment, and if that failed, try to interpret it as "\\"
> followed by a comment, neither of which would look for the correct file.
> The patch causes Clang to emit this as "\\\#" which lets GNU make find the
> right file.
>
>
>
> Regarding ninja, I don't think ninja can directly read Make-style
> dependency files?  Its syntax naively looks different enough that I'm not
> really sure that's relevant here.
>
> --paulr
>
>
>
> *From:* Sean Silva [mailto:chisophugis at gmail.com <chisophugis at gmail.com>]
> *Sent:* Friday, May 08, 2015 6:40 PM
> *To:* Robinson, Paul
> *Cc:* Nico Weber; reviews+D9208+public+0ac91376ffbd3e34 at reviews.llvm.org;
> cfe-commits at cs.uiuc.edu
> *Subject:* Re: [PATCH] Fix dependency file escaping
>
>
>
>
>
>
>
> On Fri, May 8, 2015 at 5:57 PM, Robinson, Paul <
> Paul_Robinson at playstation.sony.com> wrote:
>
> If a program (say, ninja) tries to be compatible with gnu make's depfile
> parsing, it would previously convert "\ " to a space from what I
> understand. Now it's going to get "\\\ " and think that that's "\ ".
>
>
>
> You're mixing things up.  #include "\ " would be converted by gcc to "\\ "
> (because it escapes the space but not the backslash) which would be
> de-escaped by GNU Make as "\" followed by a space delimiter.
>
> Now Clang will give it "\\\ " which will be handled as "\ " which is
> correct.
>
>
>
> Is this right? GCC and the current patch coincide on the handling of `\ `.
> The only deviation is in the `#` case, no?
>
>
>
> -- Sean Silva
>
>
>
>  (Remember that the string you hand to #include is NOT a normal C string;
> it has no escaping in it.)
>
> --paulr
>
>
>
> *From:* cfe-commits-bounces at cs.uiuc.edu [mailto:
> cfe-commits-bounces at cs.uiuc.edu] *On Behalf Of *Nico Weber
> *Sent:* Friday, May 08, 2015 4:36 PM
> *To:* reviews+D9208+public+0ac91376ffbd3e34 at reviews.llvm.org
> *Cc:* cfe-commits at cs.uiuc.edu
> *Subject:* Re: [PATCH] Fix dependency file escaping
>
>
>
> On Fri, May 8, 2015 at 4:01 PM, Paul Robinson <
> Paul_Robinson at playstation.sony.com> wrote:
>
> In http://reviews.llvm.org/D9208#169446, @thakis wrote:
>
> > Does gcc intend to fix this soon? Isn't being compatible with gcc
> important
> >  than the other things?
>
>
> If gcc emitted an incorrect relocation, would you argue that it's
> important to be compatible with gcc?  Even if you could not point to any
> linker that handled that buggy relocation in a reasonable way?
>
>
>
> 'course not, but that's not the case here. If a program (say, ninja) tries
> to be compatible with gnu make's depfile parsing, it would previously
> convert "\ " to a space from what I understand. Now it's going to get "\\\
> " and think that that's "\ ". So this is breaking backwards compat of clang
> with itself.
>
>
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>
>
>
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20150511/7a46d608/attachment.html>


More information about the cfe-commits mailing list