This is the mail archive of the
mailing list for the GCC project.
Re: [patch] diagnostics branch
- From: Richard Henderson <rth at redhat dot com>
- To: Aldy Hernandez <aldyh at redhat dot com>
- Cc: gcc-patches at gcc dot gnu dot org
- Date: Mon, 08 Jun 2009 19:26:56 -0700
- Subject: Re: [patch] diagnostics branch
- References: <20090608182819.GA17925@redhat.com>
@@ -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"),
@@ -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,
- 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
- 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
= build_array_type (gcov_type_node, NULL_TREE);
- = 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
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;
for_paran_loc is incorrect, since it's looking at the FOR token.
Though, really, what's the point of distingushing that token?