| Message ID | 20160203200726.GF3017@tucnak.redhat.com |
|---|---|
| State | New |
| Headers | show |
On Wed, 2016-02-03 at 21:07 +0100, Jakub Jelinek wrote: > Hi! > > As range->m_caret.m_{line,column} is only initialized if > range->m_show_caret_p is true, we really shouldn't be looking at > those > fields otherwise. > Fixed thusly, bootstrapped/regtested on x86_64-linux and i686-linux, > ok for > trunk? I'm not a reviewer, but fwiw the change looks good to me; thanks. [the uninitialized data is coming from: gcc_rich_location::add_expr, which leaves m_show_caret_p as false, and doesn't bother initializing m_caret].
On 02/03/2016 01:25 PM, David Malcolm wrote: > On Wed, 2016-02-03 at 21:07 +0100, Jakub Jelinek wrote: >> Hi! >> >> As range->m_caret.m_{line,column} is only initialized if >> range->m_show_caret_p is true, we really shouldn't be looking at >> those >> fields otherwise. >> Fixed thusly, bootstrapped/regtested on x86_64-linux and i686-linux, >> ok for >> trunk? > > I'm not a reviewer, but fwiw the change looks good to me; thanks. > > [the uninitialized data is coming from: > gcc_rich_location::add_expr, which leaves m_show_caret_p as false, and > doesn't bother initializing m_caret]. Given you know this code better than anyone, that should carry enough weight to be an approval, even if we haven't gone through the formal process of appointing you as a reviewer for these bits. jeff >
--- gcc/diagnostic-show-locus.c.jj 2016-01-26 20:50:26.000000000 +0100 +++ gcc/diagnostic-show-locus.c 2016-02-03 14:12:30.472706582 +0100 @@ -722,9 +722,10 @@ layout::get_state_at_point (/* Inputs. /* Are we at the range's caret? is it visible? */ out_state->draw_caret_p = false; - if (row == range->m_caret.m_line + if (range->m_show_caret_p + && row == range->m_caret.m_line && column == range->m_caret.m_column) - out_state->draw_caret_p = range->m_show_caret_p; + out_state->draw_caret_p = true; /* Within a multiline range, don't display any underline in any leading or trailing whitespace on a line. --- gcc/testsuite/gcc.dg/pr69627.c.jj 2016-02-03 14:21:17.063450583 +0100 +++ gcc/testsuite/gcc.dg/pr69627.c 2016-02-03 14:28:31.765465915 +0100 @@ -0,0 +1,27 @@ +/* PR c/69627 */ +/* { dg-do compile } */ +/* { dg-options "-fdiagnostics-show-caret" } */ + +void +foo () +{ + float t[2] = { 1, 2 }; + int const *s = 0; + t[1] / s; /* { dg-error "invalid operands to binary /" } */ +/* { dg-begin-multiline-output "" } + t[1] / s; + ~~~~ ^ + { dg-end-multiline-output "" } */ +} + +void +bar () +{ + float t[2] = { 1, 2 }; + int const *s[2] = { 0, 0 }; + t[1] / s[0]; /* { dg-error "invalid operands to binary /" } */ +/* { dg-begin-multiline-output "" } + t[1] / s[0]; + ~~~~ ^ ~~~~ + { dg-end-multiline-output "" } */ +}
Hi! As range->m_caret.m_{line,column} is only initialized if range->m_show_caret_p is true, we really shouldn't be looking at those fields otherwise. Fixed thusly, bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk? 2016-02-03 Jakub Jelinek <jakub@redhat.com> PR c/69627 * diagnostic-show-locus.c (layout::get_state_at_point): Don't read range->m_caret fields if range->m_show_caret_p is false. * gcc.dg/pr69627.c: New test. Jakub