This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: make grokfield handle locations


> Is your plan then to fix this fundamental breakage by eliminating the 
> input_location global (making every diagnostic explicitly use the location 
> of some token or range of tokens) instead of having it updated correctly 
> on change of column?

Yes.

> Patches still need a test that demonstrates that they did something.  
> This may mean explicitly including "column: diagnostic" in the test until 
> there's a better way in the test harness.  As the patch changes two 
> diagnostics, each of them should be covered in the test.  Modifying 
> existing tests of these diagnostics to check column numbers is still a 
> suitable approach.

It turns out we can't use "{dg-warning "COL:some warning message}"
because of the way gcc-dg.exp:process-message works.  The function
process-message prepends "warning:" to the regex, so we end up looking
for "warning:.*COL:some warning message" when what we actually want to
search for ":COL: warning:.*some warning message".

So, there is no way of testing for columns under the current
infrastructure.  Lucky for us, process-message() is in gcc-dg.exp, which
is under our control, not dejagnu's.

I am attaching a patch to our testsuite harness that will allow for
testing columns with:

	{ dg-{error,warning} "COL:rest of message here" }

It is the least invasive, consistent solution I have found.  

I dislike the suggestion of:

	{ dg-warning "does not declare anything" "" { target *-*-* } "COL" }

because it's overly verbose.  However, at this point I just want to fix
location information throughout the compiler, and not spend the rest of
the week coming up with a testsuite solution that will please everyone.

If anyone has a solution that is compliant with everything and everyone,
and they're willing to code it, I'll gladly use it.

How does this look?  It tests both diagnostics changed as you have
requested.

Aldy

	* c-tree.h (grokfield): New argument.
	* c-decl.c (grokfield): Handle new location argument.
	* c-parser.c (c_parser_struct_declaration): Pass location to
	grokfield.
 testsuite/
	* gcc.dg/20011008-1.c: Test column.
	* gcc.dg/20080820.c: New.
	* lib/gcc.exp (gcc_target_compile): Remove -fno-show-column.
	* lib/gcc-dg.exp (process-message): Handle columns.

Index: testsuite/gcc.dg/20011008-1.c
===================================================================
--- testsuite/gcc.dg/20011008-1.c	(revision 139207)
+++ testsuite/gcc.dg/20011008-1.c	(working copy)
@@ -1,7 +1,7 @@
 /* { dg-do compile } */
-/* { dg-options "-O0" } */
+/* { dg-options "-O0 -fshow-column" } */
 
-struct { int; int q; } a; /* { dg-warning "does not declare anything" } */
+struct { int; int q; } a; /* { dg-warning "13:does not declare anything" } */
 struct { union {int x;}; int q; } b;
 struct { struct {int x;}; int q; } c;
 union { union {int x;}; int q; } d;
Index: testsuite/gcc.dg/20080820.c
===================================================================
--- testsuite/gcc.dg/20080820.c	(revision 0)
+++ testsuite/gcc.dg/20080820.c	(revision 0)
@@ -0,0 +1,4 @@
+/* { dg-do compile } */
+/* { dg-options "-fshow-column -fms-extensions -pedantic" } */
+
+struct { struct a { int x; }; int bar; } hot; /* { dg-warning "29:ISO C doesn't support unnamed" } */
Index: testsuite/lib/gcc.exp
===================================================================
--- testsuite/lib/gcc.exp	(revision 139207)
+++ testsuite/lib/gcc.exp	(working copy)
@@ -150,7 +150,6 @@ proc gcc_target_compile { source dest ty
     if [target_info exists gcc,timeout] {
 	lappend options "timeout=[target_info gcc,timeout]"
     }
-    lappend options "additional_flags=-fno-show-column"
     lappend options "compiler=$GCC_UNDER_TEST"
     set options [dg-additional-files-options $options $source]
     return [target_compile $source $dest $type $options]
Index: testsuite/lib/gcc-dg.exp
===================================================================
--- testsuite/lib/gcc-dg.exp	(revision 139207)
+++ testsuite/lib/gcc-dg.exp	(working copy)
@@ -636,10 +636,20 @@ proc process-message { msgproc msgprefix
     # it match a single line.
     set newentry [lindex ${dg-messages} end]
     set expmsg [lindex $newentry 2]
-    set expmsg "$msgprefix\[^\n]*$expmsg"
+
+    # If we have a column...
+    if [regexp "^(\[0-9\]+):" $expmsg "" column] {
+	# Remove "COLUMN:"
+	regsub "^\[0-9\]+:" $expmsg "" expmsg
+
+	# Include the column in the search expression.
+	set expmsg "$column: $msgprefix\[^\n]*$expmsg"
+    } else {
+	set expmsg "$msgprefix\[^\n]*$expmsg"
+    }
+
     set newentry [lreplace $newentry 2 2 $expmsg]
     set dg-messages [lreplace ${dg-messages} end end $newentry]
-    verbose "process-message:\n${dg-messages}" 2
 }
 
 # Look for messages that don't have standard prefixes.
Index: c-tree.h
===================================================================
--- c-tree.h	(revision 139207)
+++ c-tree.h	(working copy)
@@ -475,8 +475,8 @@ extern tree finish_enum (tree, tree, tre
 extern void finish_function (void);
 extern tree finish_struct (tree, tree, tree);
 extern struct c_arg_info *get_parm_info (bool);
-extern tree grokfield (struct c_declarator *, struct c_declspecs *,
-		       tree, tree *);
+extern tree grokfield (location_t, struct c_declarator *,
+		       struct c_declspecs *, tree, tree *);
 extern tree groktypename (struct c_type_name *);
 extern tree grokparm (const struct c_parm *);
 extern tree implicitly_declare (tree);
Index: c-decl.c
===================================================================
--- c-decl.c	(revision 139207)
+++ c-decl.c	(working copy)
@@ -5342,12 +5342,15 @@ start_struct (enum tree_code code, tree 
    WIDTH is non-NULL for bit-fields only, and is an INTEGER_CST node.
    DECL_ATTRS is as for grokdeclarator.
 
+   LOC is the location of the structure component.
+
    This is done during the parsing of the struct declaration.
    The FIELD_DECL nodes are chained together and the lot of them
    are ultimately passed to `build_struct' to make the RECORD_TYPE node.  */
 
 tree
-grokfield (struct c_declarator *declarator, struct c_declspecs *declspecs,
+grokfield (location_t loc,
+	   struct c_declarator *declarator, struct c_declspecs *declspecs,
 	   tree width, tree *decl_attrs)
 {
   tree value;
@@ -5393,10 +5396,11 @@ grokfield (struct c_declarator *declarat
 	}
       if (!ok)
 	{
-	  pedwarn (0, "declaration does not declare anything");
+	  pedwarn_at (loc, 0, "declaration does not declare anything");
 	  return NULL_TREE;
 	}
-      pedwarn (OPT_pedantic, "ISO C doesn%'t support unnamed structs/unions");
+      pedwarn_at (loc, OPT_pedantic,
+		  "ISO C doesn%'t support unnamed structs/unions");
     }
 
   value = grokdeclarator (declarator, declspecs, FIELD, false,
Index: c-parser.c
===================================================================
--- c-parser.c	(revision 139207)
+++ c-parser.c	(working copy)
@@ -1966,7 +1966,9 @@ c_parser_struct_declaration (c_parser *p
 	     structs or unions (which is [a] useful and [b] supports
 	     MS P-SDK).  */
 	  tree attrs = NULL;
-	  ret = grokfield (build_id_declarator (NULL_TREE), specs,
+
+	  ret = grokfield (c_parser_peek_token (parser)->location,
+			   build_id_declarator (NULL_TREE), specs,
 			   NULL_TREE, &attrs);
 	  if (ret)
 	    decl_attributes (&ret, attrs, 0);
@@ -2009,7 +2011,8 @@ c_parser_struct_declaration (c_parser *p
 	    }
 	  if (c_parser_next_token_is_keyword (parser, RID_ATTRIBUTE))
 	    postfix_attrs = c_parser_attributes (parser);
-	  d = grokfield (declarator, specs, width, &all_prefix_attrs);
+	  d = grokfield (c_parser_peek_token (parser)->location,
+			 declarator, specs, width, &all_prefix_attrs);
 	  decl_attributes (&d, chainon (postfix_attrs,
 					all_prefix_attrs), 0);
 	  TREE_CHAIN (d) = decls;


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]