[PATCH] PR c/68473: sanitize source range-printing within certain macro expansions

[David Malcolm]

On Mon, 2015-11-23 at 18:59 +0100, Bernd Schmidt wrote:
> On 11/23/2015 06:52 PM, David Malcolm wrote:
> > This patch fixes PR c/68473 by bulletproofing the new
> > diagnostic_show_locus implementation against ranges that finish before
> > they start (which can happen when using the C preprocessor), falling
> > back to simply printing a caret.
> 
> Hmm, wouldn't it be better to avoid such a situation? Can you describe a 
> bit more how exactly the macro expansion caused such a situation?

The issue is here:

     1	/* { dg-options "-fdiagnostics-show-caret -mno-fp-ret-in-387" } */
     2	
     3	extern long double fminl (long double __x, long double __y);
     4	
     5	#define TEST_EQ(FUNC) do { \
     6	  if ((long)FUNC##l(xl,xl) != (long)xl) \
     7	    return; \
     8	  } while (0)
     9	
    10	void
    11	foo (long double xl)
    12	{
    13	  TEST_EQ (fmin); /* { dg-error "x87 register return with x87 disabled" } */
    14	}


    16	/* { dg-begin-multiline-output "" }
    17	   TEST_EQ (fmin);
    18	            ^
    19	   { dg-end-multiline-output "" } */
    20	
    21	/* { dg-begin-multiline-output "" }
    22	   if ((long)FUNC##l(xl,xl) != (long)xl) \
    23	             ^~~~
    24	   { dg-end-multiline-output "" } */

An error is emitted whilst expanding the macro at line 13, at
input_location.

This is at the expansion of this function call:

   fminl (xl, xl)

Normally we'd emit a source range like this for a function call:

   fminl (xl, xl)
   ^~~~~~~~~~~~~~

However, once we fully resolve locations, the "fmin" part of "fminl"
appears at line 13 here:

    13	  TEST_EQ (fmin);
                   ^~~~

giving the location of the caret, and start of the range, whereas the
rest of the the call is spelled here:

     6	  if ((long)FUNC##l(xl,xl) != (long)xl) \
                           ~~~~~~~

where the close paren gives the end of the range.

It would be wrong to try to print the whole range (anything might be
between lines 6 and 13).

In theory we could attempt to try to handle this kind of thing by
looking at the macro expansions, and to print something like:

    13	  TEST_EQ (fmin);
                   ^~~~
     6	  if ((long)FUNC##l(xl,xl) != (long)xl) \
                          ~~~~~~~~

or whatnot, but that strikes me as error-prone at this stage.


The patch instead detects such a situation, and tries to handle things
gracefully by falling back to simply printing a caret, without any
underlines:

pr68473-1.c: In function ‘foo’:
pr68473-1.c:13:12: error: x87 register return with x87 disabled
   TEST_EQ (fmin);
            ^

pr68473-1.c:6:13: note: in definition of macro ‘TEST_EQ’
   if ((long)FUNC##l(xl,xl) != (long)xl) \
             ^~~~


Dave