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: [patch] diagnostics branch


@@ -833,7 +833,8 @@ dw2_force_const_mem (rtx x, bool is_publ
sprintf (ref_name, "DW.ref.%s", str);
id = get_identifier (ref_name);
- decl = build_decl (VAR_DECL, id, ptr_type_node);
+ decl = build_decl (input_location,
+ VAR_DECL, id, ptr_type_node);
DECL_ARTIFICIAL (decl) = 1;

For the two changes in dwarf2asm.c, I prefer the UNKNOWN_LOCATION value that you use elsewhere, since these decls are totally fake.

@@ -477,7 +477,8 @@ default_stack_protect_guard (void)
if (t == NULL)
{
- t = build_decl (VAR_DECL, get_identifier ("__stack_chk_guard"),
+ t = build_decl (UNKNOWN_LOCATION,
+ VAR_DECL, get_identifier ("__stack_chk_guard"),
ptr_type_node);
...
@@ -1324,10 +1328,12 @@ create_tinfo_types (void)
{
tree field, fields;
- field = build_decl (FIELD_DECL, NULL_TREE, const_ptr_type_node);
+ field = build_decl (BUILTINS_LOCATION,
+ FIELD_DECL, NULL_TREE, const_ptr_type_node);

So what exactly is BUILTINS_LOCATION supposed to represent? An actual built-in like __builtin_memcpy, or something that may also appear in a runtime library.

If only real builtins, all these tinfo things are in libstdc++.  If
external library stuff that implements builtins, then there's at
least a half-dozen functions that are marked UNKNOWN_LOCATION.

It appears as if BUILTINS_LOCATION is treated like UNKNOWN_LOCATION
almost everywhere, except where != UNKNOWN_LOCATION, where we sometimes
assign come "current" location.  This suggests that runtime library
stuff should be considered BUILTIN.  Make sense?


@@ -3030,7 +3032,8 @@ process_template_parm (tree list, tree p
       /* A template parameter is not modifiable.  */
       TREE_CONSTANT (parm) = 1;
       TREE_READONLY (parm) = 1;
-      decl = build_decl (CONST_DECL, DECL_NAME (parm), TREE_TYPE (parm));
+      decl = build_decl (DECL_SOURCE_LOCATION (parm),
+			 CONST_DECL, DECL_NAME (parm), TREE_TYPE (parm));
       TREE_CONSTANT (decl) = 1;
       TREE_READONLY (decl) = 1;
       DECL_INITIAL (parm) = DECL_INITIAL (decl)
@@ -3059,7 +3062,8 @@ process_template_parm (tree list, tree p
 	{
 	  t = cxx_make_type (TEMPLATE_TYPE_PARM);
 	  /* parm is either IDENTIFIER_NODE or NULL_TREE.  */
-	  decl = build_decl (TYPE_DECL, parm, t);
+	  decl = build_decl (input_location,
+			     TYPE_DECL, parm, t);

Seems like the both of these should use the same location. This catches my eye because I would think that essentially no where in pt.c should use input_location, since almost all of it is template substitution and we should be copying someone else's location.


@@ -3328,7 +3329,7 @@ finalize_nrv_r (tree* tp, int* walk_subt
 	init = build2 (INIT_EXPR, void_type_node, dp->result,
 		       DECL_INITIAL (dp->var));
       else
-	init = build_empty_stmt ();
+	init = build_empty_stmt (*EXPR_LOCUS (*tp));

Is there a plan to remove EXPR_LOCUS in favour of EXPR_LOCATION, now that we've ditched the locus struct? In particular, should you be adding new uses of EXPR_LOCUS?

@@ -169,7 +169,7 @@ lower_function_body (void)
tree disp_label, disp_var, arg;
/* Build 'DISP_LABEL:' and insert. */
- disp_label = create_artificial_label ();
+ disp_label = create_artificial_label (cfun->function_end_locus);
...
 lower_builtin_setjmp (gimple_stmt_iterator *gsi)
 {
   gimple stmt = gsi_stmt (*gsi);
-  tree cont_label = create_artificial_label ();
-  tree next_label = create_artificial_label ();
+  tree cont_label = create_artificial_label (UNKNOWN_LOCATION);
+  tree next_label = create_artificial_label (UNKNOWN_LOCATION);

Surely these labels should be at the location of the setjmp call; after all, they're part of it's implementation.


+++ gcc/tree-eh.c (working copy)
@@ -499,7 +499,7 @@ replace_goto_queue_cond_clause (tree *tp
return;
}
- label = create_artificial_label ();
+ label = create_artificial_label (UNKNOWN_LOCATION);

For future work, most of the labels created in tree-eh.c can be associated with the locus of some EH statement that we're expanding. But since UNKNOWN is not a regression from the no information that we were emitting before, I don't think this should hold up merging.

Similarly for most of gimplify.c.

@@ -5533,7 +5580,8 @@ digest_init (tree type, tree init, tree {
if (TREE_CODE (inside_init) == STRING_CST
|| TREE_CODE (inside_init) == COMPOUND_LITERAL_EXPR)
- inside_init = array_to_pointer_conversion (inside_init);
+ inside_init = array_to_pointer_conversion
+ (input_location, inside_init);

Are there any valid uses of input_location within digest_init, which is passed a location_t parameter?

@@ -418,7 +418,8 @@ coverage_counter_alloc (unsigned counter
       tree gcov_type_array_type
         = build_array_type (gcov_type_node, NULL_TREE);
       tree_ctr_tables[counter]
-        = build_decl (VAR_DECL, NULL_TREE, gcov_type_array_type);
+        = build_decl (input_location,
+		      VAR_DECL, NULL_TREE, gcov_type_array_type);

Surely UNKNOWN (or BUILTIN) is better for most everything in coverage.c. As far as I can see, most of the locations here are for the stuff that we use to communicate with gcov.

+++ gcc/tree-switch-conversion.c (working copy)
@@ -517,7 +517,7 @@ build_one_array (gimple swtch, int num, ctor = build_constructor (array_type, info.constructors[num]);
TREE_CONSTANT (ctor) = true;
- decl = build_decl (VAR_DECL, NULL_TREE, array_type);
+ decl = build_decl (input_location, VAR_DECL, NULL_TREE, array_type);

Surely grep input_location tree-*.c should not match anything? In particular, this usage should be the gimple_location of the switch statement.

   if (c_parser_require (parser, CPP_OPEN_PAREN, "expected %<(%>"))
     {
+      switch_cond_loc = c_parser_peek_token (parser)->location;
       expr = c_parser_expression (parser).value;

Why did you need to record the switch condition explicitly, rather than examining EXPR_LOCATION (expr)? There are lots of other places around the parser where you do similar things, so I assume there's a good reason but I can't think what it might be.

+  location_t loc = c_parser_peek_token (parser)->location;
+  location_t for_paren_loc = c_parser_peek_token (parser)->location;
   gcc_assert (c_parser_next_token_is_keyword (parser, RID_FOR));
-  loc = c_parser_peek_token (parser)->location;
   c_parser_consume_token (parser);

for_paran_loc is incorrect, since it's looking at the FOR token. Though, really, what's the point of distingushing that token?


r~



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