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?
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. */
