C++ PATCH for c++/70449 (ICE when printing a filename of unknown location)

This test ICEs since the addition of the assert in pp_string which ensures that
we aren't trying to print an empty string.  But that's what happens here, the
location is actually UNKNOWN_LOCATION, so LOCATION_FILE on that yields null.
Fixed byt not trying to print the filename of UNKNOWN_LOCATION.

This isn't really related to the bogus -Wreturn-type warning as I initially
thought.  (With -Wreturn-type we say that F is missing a return statement,
which obviously isn't correct -- I'll open a separate PR for that.)
We might ICE anytime we call print_instantiation_full_context with location
that is in fact UNKNOWN_LOCATION.

Bootstrapped/regtested on x86_64-linux, ok for trunk?

2016-03-30  Marek Polacek  <polacek@redhat.com>

	PR c++/70449
	* error.c (print_instantiation_full_context): Don't print the filename
	for UNKNOWN_LOCATIONs.

	* g++.dg/cpp0x/constexpr-70449.C: New test.


	Marek

OK.

Jason

On 30/03/16 17:14, Marek Polacek wrote:
> This test ICEs since the addition of the assert in pp_string which ensures that
> we aren't trying to print an empty string.  But that's what happens here, the
> location is actually UNKNOWN_LOCATION, so LOCATION_FILE on that yields null.
> Fixed byt not trying to print the filename of UNKNOWN_LOCATION.

How it can be UNKNOWN_LOCATION? It has to be somewhere in the input file!

This is what Clang prints:

prog.cc:5:3: warning: type definition in a constexpr function is a C++14 
extension [-Wc++14-extensions]
   enum E { a = f<0> () };
   ^

This is what we print:

In instantiation of 'constexpr int f() [with int <anonymous> = 0]':
prog.cc:5:22:   required from here
cc1plus: error: body of constexpr function 'constexpr int f() [with int 
<anonymous> = 0]' not a return-statement

This is very broken: The fact that input_location is UNKNOWN_LOCATION makes us 
print "cc1plus" in the error.

Even if we accept the broken location for now (adding some FIXME to the code 
would help the next person to realise this is not normal), if LOCATION_FILE() 
is NULL, we should print "progname" like diagnostic_build_prefix() does. 
Moreover, the filename string should be built with file_name_as_prefix() to get 
correct coloring.

Something like:

       const char *filename = LOCATION_FILE (location);
       /* FIXME: Somehow we may get UNKNOWN_LOCATION here: See 
g++.dg/cpp0x/constexpr-70449.C */
       if (filename == NULL) filename = progname;
       const char * prefix = file_name_as_prefix (context, filename);

	pp_verbatim (context->printer,
		     TREE_CODE (p->decl) == TREE_LIST
		     ? _("%s: In substitution of %qS:\n")
		     : _("%s: In instantiation of %q#D:\n"),
		     prefix, p->decl);
       free (prefix);

The above also avoids adding yet another slightly different new string for 
translation.

Cheers,

Manuel.

On 30 March 2016 at 23:42, Manuel López-Ibáñez <lopezibanez@gmail.com> wrote:
> On 30/03/16 17:14, Marek Polacek wrote:
>>
>> This test ICEs since the addition of the assert in pp_string which ensures
>> that
>> we aren't trying to print an empty string.  But that's what happens here,
>> the
>> location is actually UNKNOWN_LOCATION, so LOCATION_FILE on that yields
>> null.
>> Fixed byt not trying to print the filename of UNKNOWN_LOCATION.

> Even if we accept the broken location for now (adding some FIXME to the code
> would help the next person to realise this is not normal), if
> LOCATION_FILE() is NULL, we should print "progname" like
> diagnostic_build_prefix() does. Moreover, the filename string should be
> built with file_name_as_prefix() to get correct coloring.

Even better: Use "f ? f : progname" in file_name_as_prefix() and
simplify the code to:

      /* FIXME: Somehow we may get UNKNOWN_LOCATION here: See
g++.dg/cpp0x/constexpr-70449.C */
      const char * prefix = file_name_as_prefix (context,
LOCATION_FILE (location));
        pp_verbatim (context->printer,
                     TREE_CODE (p->decl) == TREE_LIST
                     ? _("%s: In substitution of %qS:\n")
                     : _("%s: In instantiation of %q#D:\n"),
                     prefix, p->decl);
      free (prefix);

Fixes the ICE, adds colors, mentions the broken location and does not
add extra strings.

Cheers,

Manuel.