On Tue, 2015-12-29 at 12:53 -0800, Mike Stump wrote:
> On Sep 25, 2015, at 1:11 PM, David Malcolm <dmalc...@redhat.com>
> wrote:
> > +layout::layout (diagnostic_context * context,
> >> + const diagnostic_info *diagnostic)
> >>
> >> [...]
> >>
> >> + if (loc_range->m_finish.file != m_exploc.file)
> >> + continue;
> >> + if (loc_range->m_show_caret_p)
> >> + if (loc_range->m_finish.file != m_exploc.file)
> >> + continue;
> >>
> >> I think the second if clause is redundant.
> >
> > Good catch; thanks.
>
> And one other nit. You don’t validate that the range finishes on or
> after the start. Later in the code, you require this to be the case:
>
> bool
> layout_range::contains_point (int row, int column) const
> {
> gcc_assert (m_start.m_line <= m_finish.m_line);
>
> this code cannot tolerate a range with that property. So, either,
> such a range should never be generated, or, if it is to be generated,
> at least we should declare it awkward. The below patch declares it to
> be awkward. Without this, we ice on completely sane and normal code:
>
> #define max(i, j) sel((j), (i), (i) < (j))
> yu = max(a2, a3);
>
> giving a valid warning. In the code, we start on the last line, and
> finish on the first line. The underlying problem is that we don’t
> track macro contexts properly. sel is a compiler built-in, so, it
> might be funnier that just a normal function call. This is from a
> trunk compiler from 20151201.
>
> So, I was wondering if the problem has been fixed, or, if we should
> put the below in now, or, would you prefer to try and do up the
> changes to better track macro expansions?
This is PR 68473.
I committed a workaround for it, similar to your one, as r231919 on
2015-12-22; I've been experimenting with a "deeper" fix for it that
would better respect macro expansions, but that might have to wait until
gcc 7.
> diff --git a/gcc/diagnostic-show-locus.c b/gcc/diagnostic-show-locus.c
> index 9e51b95..bea8423 100644
> --- a/gcc/diagnostic-show-locus.c
> +++ b/gcc/diagnostic-show-locus.c
> @@ -455,6 +455,9 @@ layout::layout (diagnostic_context * context,
> if (loc_range->m_show_caret_p)
> if (loc_range->m_caret.file != m_exploc.file)
> continue;
> + /* A range that finishes before it starts is awkward. */
> + if (loc_range->m_start.line > loc_range->m_finish.line)
> + continue;
>
> /* Passed all the tests; add the range to m_layout_ranges so
> that
> it will be printed. */
>
