<div style="font-family: arial, helvetica, sans-serif"><font size="2">Doh, sorry for missing this last patch.<div><br></div><div>Looks pretty good. As discussed in person, you may want to sink the arg mutation below the PrintJob... Not sure.</div>
<div><br></div><div>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.</div>
<div><br><div class="gmail_quote">On Fri, Jun 22, 2012 at 4:27 PM, David Blaikie <span dir="ltr"><<a href="mailto:dblaikie@gmail.com" target="_blank">dblaikie@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
[bump]<br>
<div class="HOEnZb"><div class="h5"><br>
On Mon, Jun 18, 2012 at 10:54 AM, David Blaikie <<a href="mailto:dblaikie@gmail.com">dblaikie@gmail.com</a>> wrote:<br>
> [bump for Chandler so we can finish off this change]<br>
><br>
> On Thu, Jun 14, 2012 at 4:18 PM, David Blaikie <<a href="mailto:dblaikie@gmail.com">dblaikie@gmail.com</a>> wrote:<br>
>> On Thu, Jun 14, 2012 at 1:07 PM, Chandler Carruth <<a href="mailto:chandlerc@google.com">chandlerc@google.com</a>> wrote:<br>
>>> On Thu, Jun 14, 2012 at 1:05 PM, David Blaikie <<a href="mailto:dblaikie@gmail.com">dblaikie@gmail.com</a>> wrote:<br>
>>>><br>
>>>> On Thu, Jun 14, 2012 at 11:08 AM, Chandler Carruth <<a href="mailto:chandlerc@google.com">chandlerc@google.com</a>><br>
>>>> wrote:<br>
>>>> > The driver support for rewrite includes LGTM, commit away.<br>
>>>><br>
>>>> r158463<br>
>>>><br>
>>>> > Regarding the crash reporting, that looks too easy! ;]<br>
>>>><br>
>>>> It does, doesn't it...<br>
>>>><br>
>>>> > Does anything need to be changed in the reproduction script to make it<br>
>>>> > use the include-rewritten<br>
>>>> > file instead of a fully preprocessed file?<br>
>>>><br>
>>>> Ah, I hadn't actually tried the repro script file - I tend to just run<br>
>>>> the crashers myself (but of course then I don't get the extra flags,<br>
>>>> etc, from the repro).<br>
>>>><br>
>>>> The script is generated based on the file name reported as the output<br>
>>>> file by the compilation command, as I understand/read it - so it's not<br>
>>>> referring to a non-existent file (& the file name remains the same in<br>
>>>> any case).<br>
>>>><br>
>>>> >  Do we need to change the filename?<br>
>>><br>
>>><br>
>>> I should have said filename *extension*.<br>
>>><br>
>>>><br>
>>>> It seems to work fine (reckon it'd be appropriate to have a test case<br>
>>>> for this that actually executes the crash report script file? that<br>
>>>> seems a little nasty, but with some value). The -cc1 command includes<br>
>>>> an explicit -x so I don't think there's any file extension based<br>
>>>> language detection (so it doesn't try to skip preprocessing, for<br>
>>>> example).<br>
>>><br>
>>><br>
>>> Wild. I was thinking the '.ii' extension might cause it to do the wrong<br>
>>> thing.<br>
>><br>
>> Yeah, so did I - but it really does seem to still properly preprocess<br>
>> the macros & compile just fine.<br>
>><br>
>>> I think we should still fix the extension to be the same as the original file.<br>
>><br>
>> Yeah, it seems reasonable to me. See attached.<br>
</div></div></blockquote></div><br></div></font></div>