On Wed, Mar 30, 2016 at 11:51:26PM +0100, Manuel López-Ibáñez wrote:
> On 30 March 2016 at 23:42, Manuel López-Ibáñez <lopeziba...@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:
It seems wrong to me to just add that. I wonder if the function shouldn't
use diagnostic_get_location_text instead which also handles "<built-in>" etc.
In any case I'd rather not touch the diagnostic stuff in the scope of this
patch (which wouldn't probably be suitable for stage4 anyway).
> /* 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.
It fixes the ICE, but I don't see any more colors than with my patch.
Moreover your suggestion would print
cc1plus: : In instantiation of ‘constexpr int f() [with int I = 0]’:
, not sure how's that better than simply saying
In instantiation of ‘constexpr int f() [with int I = 0]’:
I.e. adding "cc1plus" to the output doesn't seem like an improvement.
Anyway, here's a patch which uses file_name_as_prefix.
Bootstrapped/regtested on x86_64-linux, ok for trunk?
2016-03-31 Marek Polacek <pola...@redhat.com>
PR c++/70449
* error.c (print_instantiation_full_context): Build prefix by using
file_name_as_prefix.
* g++.dg/cpp0x/constexpr-70449.C: New test.
diff --git gcc/cp/error.c gcc/cp/error.c
index aa5fd41..14b0205 100644
--- gcc/cp/error.c
+++ gcc/cp/error.c
@@ -3312,12 +3312,15 @@ print_instantiation_full_context (diagnostic_context
*context)
if (p)
{
+ char *prefix = file_name_as_prefix (context, LOCATION_FILE (location)
+ ? LOCATION_FILE (location)
+ : progname);
pp_verbatim (context->printer,
TREE_CODE (p->decl) == TREE_LIST
- ? _("%s: In substitution of %qS:\n")
- : _("%s: In instantiation of %q#D:\n"),
- LOCATION_FILE (location),
- p->decl);
+ ? _("%sIn substitution of %qS:\n")
+ : _("%sIn instantiation of %q#D:\n"),
+ prefix, p->decl);
+ free (prefix);
location = p->locus;
p = p->next;
diff --git gcc/testsuite/g++.dg/cpp0x/constexpr-70449.C
gcc/testsuite/g++.dg/cpp0x/constexpr-70449.C
index e69de29..bc5dd71 100644
--- gcc/testsuite/g++.dg/cpp0x/constexpr-70449.C
+++ gcc/testsuite/g++.dg/cpp0x/constexpr-70449.C
@@ -0,0 +1,12 @@
+// PR c++/70449
+// { dg-do compile { target c++11 } }
+
+template <int>
+constexpr
+int f (void)
+{
+ enum E { a = f<0> () };
+ return 0;
+}
+
+// { dg-error "body of constexpr function" "" { target { ! c++14 } } 0 }
Marek
