[cfe-commits] [PATCH] Fix crash printing diagnostic range spanning macros

[Eli Friedman]

On Mon, Oct 29, 2012 at 4:07 PM, Eli Friedman <eli.friedman at gmail.com> wrote:
> On Thu, Oct 25, 2012 at 6:09 PM, Eli Friedman <eli.friedman at gmail.com> wrote:
>> On Thu, Oct 25, 2012 at 3:32 PM, Chandler Carruth <chandlerc at google.com> wrote:
>>> On Thu, Oct 25, 2012 at 2:57 PM, Eli Friedman <eli.friedman at gmail.com> wrote:
>>>> On Thu, Oct 25, 2012 at 1:27 PM, Chandler Carruth <chandlerc at google.com> wrote:
>>>>> On Thu, Oct 25, 2012 at 9:38 AM, Jordan Rose <jordan_rose at apple.com> wrote:
>>>>>> I think that's the best we can do. Even if the range had the beginning
>>>>>> before the end (say, by trying to highlight the entirety of both macros), it
>>>>>> wouldn't be "correct".  We should not show ranges that don't correspond to
>>>>>> something meaningful in the text.
>>>>>
>>>>> I actually think we can do a bit better.
>>>>
>>>> Yes, we could completely change what we display, but I'm not really
>>>> interested in embarking on a large architectural project at the
>>>> moment.
>>>>
>>>>>> ...though arguably we could show a line with all macros expanded, and put
>>>>>> the range there. But that's a big change in what you expect from diagnostic
>>>>>> printing, and it wouldn't work in IDEs anyway.
>>>>>
>>>>> We get pretty close with the macro backtrace. I have sometimes
>>>>> wondered if we should start the error with a synthetic preprocessed
>>>>> snippet, and then give the code the user wrote in the first note, and
>>>>> descend through the macro expansions in subsequent notes.
>>>>> Alternatively, we could add a final note to the macro backtrace that
>>>>> shows the fully preprocessed source, but that seems more likely to be
>>>>> ignored.
>>>>
>>>> Hmm, interesting; please file a bug. :)
>>>>
>>>>>>
>>>>>> Jordan
>>>>>>
>>>>>>
>>>>>> On Oct 24, 2012, at 19:36 , Eli Friedman <eli.friedman at gmail.com> wrote:
>>>>>>
>>>>>> Patch attached.  Fixes a crash on a testcase like the following:
>>>>>>
>>>>>> +#define BAD_CONDITIONAL_OPERATOR (2<3)?4:5
>>>>>> +int x = BAD_CONDITIONAL_OPERATOR+BAD_CONDITIONAL_OPERATOR;
>>>>>>
>>>>>> We try to print a source range which starts at the 5 in the first
>>>>>> expansion, and ends just after the 3 in the second expansion.
>>>>>
>>>>> My suggestion would be this:
>>>>>
>>>>> When you have a source range to highlight, and it's start or stop
>>>>> location occurs within a macro, grow it to the start (or stop, resp.)
>>>>> of the macro info's expansion location. This should be the start of
>>>>> where the macro got expanded into the code.
>>>>>
>>>>> Then, if there the diagnostic location itself is inside a macro, as
>>>>> you do the macro backtrace walk you'll need to address the fixme in
>>>>> the diagnostic code to actually walk the source ranges back through
>>>>> the macro backtrace as well, and at each level to the analogous
>>>>> transform to grow the range at that level.
>>>>
>>>> We already do this; we just don't do it correctly for the case where
>>>> the start and/or end locations come from a different expansion than
>>>> the caret.
>>>
>>> Yes, but do we do the first paragraph correctly? I think we can do the
>>> first paragraph and fix the crash/misbehavior you're talking about.
>>
>> I see what you mean.  New patch attached.
>
> Ping.

Chandler, were you planning on reviewing this?

-Eli