On Wed, May 13, 2015 at 2:23 PM, Robinson, Paul < [email protected]> wrote:
> r237296. I believe I've answered all the questions, there have been no > new comments for two days, and Sean already gave approval. > One person's approval does not override another person's concerns. Please back out the `#` part until Nico's concerns have been fully addressed. > Thanks! > > --paulr > > > > *From:* [email protected] [mailto: > [email protected]] *On Behalf Of *Robinson, Paul > *Sent:* Monday, May 11, 2015 11:17 AM > *To:* Nico Weber > > *Cc:* [email protected]; > [email protected] > *Subject:* RE: [PATCH] Fix dependency file escaping > > > > One would naively think that GNU Make ought to be the program that can > successfully parse gcc-produced depfiles, but it demonstrably does not (in > the obviously obscure corner-case of a filename that has a '#' immediately > after a '\' which, one must admit, is not inconceivable on Windows). > > So, shall we imitate gcc behavior that even the gcc people think doesn't > look right (gcc.gnu.org/bugzilla/show_bug.cgi?id=65852) and can't be > handled correctly even by GNU Make? Or shall we implement behavior that > GNU Make can parse correctly, even though it's not exactly what gcc > produces? > > > > I am more inclined to produce a depfile that can be handled correctly by > current tools than to produce one that cannot because of what even the gcc > people admit looks wrong. The definition of what a Makefile looks like > more properly belongs to GNU Make than to GCC. > > --paulr > > > > *From:* [email protected] [mailto:[email protected] <[email protected]>] *On > Behalf Of *Nico Weber > *Sent:* Monday, May 11, 2015 10:21 AM > *To:* Robinson, Paul > *Cc:* Sean Silva; [email protected]; > [email protected] > *Subject:* Re: [PATCH] Fix dependency file escaping > > > > 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 < > [email protected]> 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:* [email protected] [mailto: > [email protected]] *On Behalf Of *Robinson, Paul > *Sent:* Monday, May 11, 2015 7:58 AM > *To:* Sean Silva > *Cc:* [email protected]; > [email protected] > *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:[email protected] <[email protected]>] > *Sent:* Friday, May 08, 2015 6:40 PM > *To:* Robinson, Paul > *Cc:* Nico Weber; [email protected]; > [email protected] > *Subject:* Re: [PATCH] Fix dependency file escaping > > > > > > > > On Fri, May 8, 2015 at 5:57 PM, Robinson, Paul < > [email protected]> 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:* [email protected] [mailto: > [email protected]] *On Behalf Of *Nico Weber > *Sent:* Friday, May 08, 2015 4:36 PM > *To:* [email protected] > *Cc:* [email protected] > *Subject:* Re: [PATCH] Fix dependency file escaping > > > > On Fri, May 8, 2015 at 4:01 PM, Paul Robinson < > [email protected]> 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 > [email protected] > http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits > > > > > _______________________________________________ > cfe-commits mailing list > [email protected] > http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits > > > > _______________________________________________ > cfe-commits mailing list > [email protected] > http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits > >
_______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
