[cfe-commits] [patch] rewrite-includes crash support

[Chandler Carruth]

Doh, sorry for missing this last patch.

Looks pretty good. As discussed in person, you may want to sink the arg
mutation below the PrintJob... Not sure.

I'm concerned by the lack of test case modifications. If we don't have a
test case yet for this, lets get one and check the filename. If we do, we
should tighten it up to check the filename or figure out whats going on.

On Fri, Jun 22, 2012 at 4:27 PM, David Blaikie <dblaikie at gmail.com> wrote:

> [bump]
>
> On Mon, Jun 18, 2012 at 10:54 AM, David Blaikie <dblaikie at gmail.com>
> wrote:
> > [bump for Chandler so we can finish off this change]
> >
> > On Thu, Jun 14, 2012 at 4:18 PM, David Blaikie <dblaikie at gmail.com>
> wrote:
> >> On Thu, Jun 14, 2012 at 1:07 PM, Chandler Carruth <chandlerc at google.com>
> wrote:
> >>> On Thu, Jun 14, 2012 at 1:05 PM, David Blaikie <dblaikie at gmail.com>
> wrote:
> >>>>
> >>>> On Thu, Jun 14, 2012 at 11:08 AM, Chandler Carruth <
> chandlerc at google.com>
> >>>> wrote:
> >>>> > The driver support for rewrite includes LGTM, commit away.
> >>>>
> >>>> r158463
> >>>>
> >>>> > Regarding the crash reporting, that looks too easy! ;]
> >>>>
> >>>> It does, doesn't it...
> >>>>
> >>>> > Does anything need to be changed in the reproduction script to make
> it
> >>>> > use the include-rewritten
> >>>> > file instead of a fully preprocessed file?
> >>>>
> >>>> Ah, I hadn't actually tried the repro script file - I tend to just run
> >>>> the crashers myself (but of course then I don't get the extra flags,
> >>>> etc, from the repro).
> >>>>
> >>>> The script is generated based on the file name reported as the output
> >>>> file by the compilation command, as I understand/read it - so it's not
> >>>> referring to a non-existent file (& the file name remains the same in
> >>>> any case).
> >>>>
> >>>> >  Do we need to change the filename?
> >>>
> >>>
> >>> I should have said filename *extension*.
> >>>
> >>>>
> >>>> It seems to work fine (reckon it'd be appropriate to have a test case
> >>>> for this that actually executes the crash report script file? that
> >>>> seems a little nasty, but with some value). The -cc1 command includes
> >>>> an explicit -x so I don't think there's any file extension based
> >>>> language detection (so it doesn't try to skip preprocessing, for
> >>>> example).
> >>>
> >>>
> >>> Wild. I was thinking the '.ii' extension might cause it to do the wrong
> >>> thing.
> >>
> >> Yeah, so did I - but it really does seem to still properly preprocess
> >> the macros & compile just fine.
> >>
> >>> I think we should still fix the extension to be the same as the
> original file.
> >>
> >> Yeah, it seems reasonable to me. See attached.
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20120622/c76fd49e/attachment.html>