Re: [C++ Patch] Fix some simple location issues

[Paolo Carlini <paolo.carlini@oracle.com>]

[-- Attachment #1: Type: text/plain, Size: 1177 bytes --]

Hi,

On 07/06/2016 21:40, Jason Merrill wrote:
> For diagnostics about an initializer, I think it would be better to 
> give the location of the initializer rather than the declaration.  
> Maybe use
>
> location_t loc = EXPR_LOC_OR_LOC (init, DECL_SOURCE_LOCATION (decl));
Now that you are telling me this, I think we already briefly discussed 
it in the past... Anyway, I investigated it a little bit a couple of 
days ago and noticed that, in fact, other front-ends differ about many 
of those details, even when normally they have accurate locations. In 
some cases, for example, clang outputs the primary caret under the 
declaration and only the secondary wiggly line under the initializer, 
but only in some cases (see, for example, g++.dg/init/brace6.C, both in 
c++98 and c++11).

Anyway, in practice, for this first round of changes, I tried your 
suggestion above in 4 places, in maybe_deduce_size_from_array_init and 
in check_initializer, but in practice doesn't currently make a 
difference, at least for the testcases, for lack of usable locations. 
Certainly could in the future! The below still passes testing...

Thanks,
Paolo.

/////////////////////////////

[-- Attachment #2: patch_locs_1b --]
[-- Type: text/plain, Size: 10163 bytes --]

Index: cp/decl.c
===================================================================
--- cp/decl.c	(revision 237171)
+++ cp/decl.c	(working copy)
@@ -1393,7 +1393,7 @@ duplicate_decls (tree newdecl, tree olddecl, bool
     {
       if (DECL_INITIAL (olddecl))
 	inform (DECL_SOURCE_LOCATION (olddecl),
-		"previous definition of %q+D was here", olddecl);
+		"previous definition of %qD was here", olddecl);
       else
 	inform (DECL_SOURCE_LOCATION (olddecl),
 		"previous declaration of %qD was here", olddecl);
@@ -5266,13 +5266,16 @@ maybe_deduce_size_from_array_init (tree decl, tree
 					    do_default);
 	  if (failure == 1)
 	    {
-	      error ("initializer fails to determine size of %qD", decl);
+	      error_at (EXPR_LOC_OR_LOC (initializer,
+					 DECL_SOURCE_LOCATION (decl)),
+			"initializer fails to determine size of %qD", decl);
 	    }
 	  else if (failure == 2)
 	    {
 	      if (do_default)
 		{
-		  error ("array size missing in %qD", decl);
+		  error_at (DECL_SOURCE_LOCATION (decl),
+			    "array size missing in %qD", decl);
 		}
 	      /* If a `static' var's size isn't known, make it extern as
 		 well as static, so it does not get allocated.  If it's not
@@ -5283,7 +5286,8 @@ maybe_deduce_size_from_array_init (tree decl, tree
 	    }
 	  else if (failure == 3)
 	    {
-	      error ("zero-size array %qD", decl);
+	      error_at (DECL_SOURCE_LOCATION (decl),
+			"zero-size array %qD", decl);
 	    }
 	}
 
@@ -5322,7 +5326,8 @@ layout_var_decl (tree decl)
       /* An automatic variable with an incomplete type: that is an error.
 	 Don't talk about array types here, since we took care of that
 	 message in grokdeclarator.  */
-      error ("storage size of %qD isn%'t known", decl);
+      error_at (DECL_SOURCE_LOCATION (decl),
+		"storage size of %qD isn%'t known", decl);
       TREE_TYPE (decl) = error_mark_node;
     }
 #if 0
@@ -5345,7 +5350,8 @@ layout_var_decl (tree decl)
 	constant_expression_warning (DECL_SIZE (decl));
       else
 	{
-	  error ("storage size of %qD isn%'t constant", decl);
+	  error_at (DECL_SOURCE_LOCATION (decl),
+		    "storage size of %qD isn%'t constant", decl);
 	  TREE_TYPE (decl) = error_mark_node;
 	}
     }
@@ -5954,7 +5960,8 @@ check_array_initializer (tree decl, tree type, tre
   if (!COMPLETE_TYPE_P (complete_type (element_type)))
     {
       if (decl)
-	error ("elements of array %q#D have incomplete type", decl);
+	error_at (DECL_SOURCE_LOCATION (decl),
+		  "elements of array %q#D have incomplete type", decl);
       else
 	error ("elements of array %q#T have incomplete type", type);
       return true;
@@ -6018,7 +6025,8 @@ check_initializer (tree decl, tree init, int flags
     }
   else if (!COMPLETE_TYPE_P (type))
     {
-      error ("%q#D has incomplete type", decl);
+      error_at (DECL_SOURCE_LOCATION (decl),
+		"%q#D has incomplete type", decl);
       TREE_TYPE (decl) = error_mark_node;
       return NULL_TREE;
     }
@@ -6038,8 +6046,9 @@ check_initializer (tree decl, tree init, int flags
 	    }
 	  else if (init_len != 1 && TREE_CODE (type) != COMPLEX_TYPE)
 	    {
-	      error ("scalar object %qD requires one element in initializer",
-		     decl);
+	      error_at (EXPR_LOC_OR_LOC (init, DECL_SOURCE_LOCATION (decl)),
+			"scalar object %qD requires one element in "
+			"initializer", decl);
 	      TREE_TYPE (decl) = error_mark_node;
 	      return NULL_TREE;
 	    }
@@ -6081,9 +6090,10 @@ check_initializer (tree decl, tree init, int flags
 	    {
 	      /* Don't reshape if the class has constructors.  */
 	      if (cxx_dialect == cxx98)
-		error ("in C++98 %qD must be initialized by constructor, "
-		       "not by %<{...}%>",
-		       decl);
+		error_at (EXPR_LOC_OR_LOC (init, DECL_SOURCE_LOCATION (decl)),
+			  "in C++98 %qD must be initialized by "
+			  "constructor, not by %<{...}%>",
+			  decl);
 	    }
 	  else if (VECTOR_TYPE_P (type) && TYPE_VECTOR_OPAQUE (type))
 	    {
@@ -6175,8 +6185,11 @@ check_initializer (tree decl, tree init, int flags
 	      && DECL_INITIAL (decl)
 	      && TREE_CODE (DECL_INITIAL (decl)) == STRING_CST
 	      && PAREN_STRING_LITERAL_P (DECL_INITIAL (decl)))
-	    warning (0, "array %qD initialized by parenthesized string literal %qE",
-		     decl, DECL_INITIAL (decl));
+	    warning_at (EXPR_LOC_OR_LOC (DECL_INITIAL (decl),
+					 DECL_SOURCE_LOCATION (decl)),
+			0, "array %qD initialized by parenthesized "
+			"string literal %qE",
+			decl, DECL_INITIAL (decl));
 	  init = NULL;
 	}
     }
@@ -12528,7 +12541,7 @@ check_elaborated_type_specifier (enum tag_types ta
 	   && tag_code != typename_type)
     {
       error ("%qT referred to as %qs", type, tag_name (tag_code));
-      inform (input_location, "%q+T has a previous declaration here", type);
+      inform (location_of (type), "%qT has a previous declaration here", type);
       return error_mark_node;
     }
   else if (TREE_CODE (type) != ENUMERAL_TYPE
@@ -12535,7 +12548,7 @@ check_elaborated_type_specifier (enum tag_types ta
 	   && tag_code == enum_type)
     {
       error ("%qT referred to as enum", type);
-      inform (input_location, "%q+T has a previous declaration here", type);
+      inform (location_of (type), "%qT has a previous declaration here", type);
       return error_mark_node;
     }
   else if (!allow_template_p
Index: testsuite/g++.dg/cpp0x/constexpr-ice10.C
===================================================================
--- testsuite/g++.dg/cpp0x/constexpr-ice10.C	(revision 237171)
+++ testsuite/g++.dg/cpp0x/constexpr-ice10.C	(working copy)
@@ -4,5 +4,5 @@
 struct A
 {
   constexpr A() {}
-  static constexpr A a[2] = {};  // { dg-error "incomplete" }
+  static constexpr A a[2] = {};  // { dg-error "22:elements of array 'constexpr const A A::a \\\[2\\\]' have incomplete type" }
 };
Index: testsuite/g++.dg/cpp0x/constexpr-incomplete1.C
===================================================================
--- testsuite/g++.dg/cpp0x/constexpr-incomplete1.C	(revision 237171)
+++ testsuite/g++.dg/cpp0x/constexpr-incomplete1.C	(working copy)
@@ -2,6 +2,6 @@
 
 struct A
 {
-  static constexpr A a = 1;	// { dg-error "incomplete" }
+  static constexpr A a = 1;  // { dg-error "22:'constexpr const A A::a' has incomplete type" }
   constexpr A(int i) { }
 };
Index: testsuite/g++.dg/cpp1y/auto-fn27.C
===================================================================
--- testsuite/g++.dg/cpp1y/auto-fn27.C	(revision 237171)
+++ testsuite/g++.dg/cpp1y/auto-fn27.C	(working copy)
@@ -31,7 +31,7 @@ F<T>::bar (const G &)
 {
   auto s = I;
   typedef decltype (s) L;
-  auto u =[&](L) { auto t = foo (J::K (), 0); }; // { dg-error "" }
+  auto u =[&](L) { auto t = foo (J::K (), 0); }; // { dg-error "25:'void t' has incomplete type" }
 }
 struct B {
   typedef int G;
Index: testsuite/g++.dg/gomp/pr35751.C
===================================================================
--- testsuite/g++.dg/gomp/pr35751.C	(revision 237171)
+++ testsuite/g++.dg/gomp/pr35751.C	(working copy)
@@ -5,8 +5,8 @@
 void
 foo (int i)
 {
-  extern int a[i];	// { dg-error "storage size of" }
-  static int b[i];	// { dg-error "storage size of" }
+  extern int a[i];	// { dg-error "14:storage size of" }
+  static int b[i];	// { dg-error "14:storage size of" }
 
 #pragma omp parallel
   {
Index: testsuite/g++.dg/init/array23.C
===================================================================
--- testsuite/g++.dg/init/array23.C	(revision 237171)
+++ testsuite/g++.dg/init/array23.C	(working copy)
@@ -3,4 +3,4 @@
 //  array
 
 struct A {A();int A::* t;};
-A x[]; // { dg-error "size" }
+A x[]; // { dg-error "3:array size missing" }
Index: testsuite/g++.dg/init/array42.C
===================================================================
--- testsuite/g++.dg/init/array42.C	(revision 0)
+++ testsuite/g++.dg/init/array42.C	(working copy)
@@ -0,0 +1 @@
+char a[] = ("abc");  // { dg-warning "6:array 'a' initialized by parenthesized string literal" }
Index: testsuite/g++.dg/init/array43.C
===================================================================
--- testsuite/g++.dg/init/array43.C	(revision 0)
+++ testsuite/g++.dg/init/array43.C	(working copy)
@@ -0,0 +1,2 @@
+int a[] = 0;  // { dg-error "5:initializer fails to determine size" }
+// { dg-error "11:array must be initialized" "" { target *-*-* } 1 }
Index: testsuite/g++.dg/init/array44.C
===================================================================
--- testsuite/g++.dg/init/array44.C	(revision 0)
+++ testsuite/g++.dg/init/array44.C	(working copy)
@@ -0,0 +1 @@
+int a[] = { };  // { dg-error "5:zero-size array" } 
Index: testsuite/g++.dg/init/array45.C
===================================================================
--- testsuite/g++.dg/init/array45.C	(revision 0)
+++ testsuite/g++.dg/init/array45.C	(working copy)
@@ -0,0 +1 @@
+int a[];  // { dg-error "5:storage size" }
Index: testsuite/g++.dg/init/brace2.C
===================================================================
--- testsuite/g++.dg/init/brace2.C	(revision 237171)
+++ testsuite/g++.dg/init/brace2.C	(working copy)
@@ -3,6 +3,6 @@
 int x = { 2 };
 const char * y = { "hello" };
 int a = 2;
-int b = { 2,3 }; // { dg-error "requires one element in initializer" }
+int b = { 2,3 }; // { dg-error "5:scalar object 'b' requires one element in initializer" }
 int c = { { 2 } } ; // { dg-error "braces around scalar initializer" }
 int d = {}; // { dg-error "initializer" "" { target { ! c++11 } } }
Index: testsuite/g++.dg/init/brace6.C
===================================================================
--- testsuite/g++.dg/init/brace6.C	(revision 237171)
+++ testsuite/g++.dg/init/brace6.C	(working copy)
@@ -17,8 +17,8 @@ struct D { int c; };
 int main()
 {
    int i = { 1 };
-   int j = { 1, 2 }; /* { dg-error "requires one element" } */
-   A a = { 6 }; /* { dg-error "initialize" "" { target { ! c++11 } } } */
+   int j = { 1, 2 }; /* { dg-error "8:scalar object 'j' requires one element" } */
+   A a = { 6 }; /* { dg-error "6:in C\\+\\+98 'a' must be initialized" "" { target { ! c++11 } } } */
    B b = { 6 }; /* { dg-error "" } */
    C c = { 6 }; /* { dg-error "too many initializers" } */
    D d = { 6 };