On Tue, Apr 10, 2018 at 10:21 AM, Jason Merrill <ja...@redhat.com> wrote:
> 65821 complains that in this testcase, setting a breakpoint on main()
> ends up stopping at the line with the default argument of foo(). I
> think I agree that the confusion from this debugging experience
> outweighs the usefulness of being able to step through evaluation of a
> default argument, so this patch changes all expressions from a default
> argument to have the location of the call, rather than only calls to
> the source-location built-ins.
Oops, that broke the libstdc++ source_location/1.cc test, which needs
the same treatment for NSDMIs, which also makes sense. So, different
approach:
Tested x86_64-pc-linux-gnu, applying to trunk.
commit b1a319de39267a858e7bc1ea02ab40fdbbb8f062
Author: Jason Merrill <ja...@redhat.com>
Date: Tue Apr 10 12:45:46 2018 -0400
PR debug/65821 - wrong location for main().
* call.c (clear_location_r, convert_default_arg): Revert.
* tree.c (break_out_target_exprs): Add clear_location parm.
(struct bot_data): New.
(bot_manip): Clear location if requested.
* init.c (get_nsdmi): Pass clear_location.
diff --git a/gcc/cp/call.c b/gcc/cp/call.c
index 94226d6ea71..38b16ba991e 100644
--- a/gcc/cp/call.c
+++ b/gcc/cp/call.c
@@ -7296,21 +7296,6 @@ cxx_type_promotes_to (tree type)
return promote;
}
-/* walk_tree callback to override EXPR_LOCATION in an expression tree. */
-
-tree
-clear_location_r (tree *tp, int *walk_subtrees, void */*data*/)
-{
- if (!EXPR_P (*tp))
- {
- *walk_subtrees = 0;
- return NULL_TREE;
- }
- if (EXPR_HAS_LOCATION (*tp))
- SET_EXPR_LOCATION (*tp, input_location);
- return NULL_TREE;
-}
-
/* ARG is a default argument expression being passed to a parameter of
the indicated TYPE, which is a parameter to FN. PARMNUM is the
zero-based argument number. Do any required conversions. Return
@@ -7374,11 +7359,7 @@ convert_default_arg (tree type, tree arg, tree fn, int parmnum,
push_deferring_access_checks (dk_no_check);
/* We must make a copy of ARG, in case subsequent processing
alters any part of it. */
- arg = break_out_target_exprs (arg);
-
- /* The use of a default argument has the location of the call, not where it
- was originally written. */
- cp_walk_tree_without_duplicates (&arg, clear_location_r, NULL);
+ arg = break_out_target_exprs (arg, /*clear location*/true);
arg = convert_for_initialization (0, type, arg, LOOKUP_IMPLICIT,
ICR_DEFAULT_ARGUMENT, fn, parmnum,
diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h
index 204791e51cf..be77f56fafb 100644
--- a/gcc/cp/cp-tree.h
+++ b/gcc/cp/cp-tree.h
@@ -7058,7 +7058,7 @@ extern tree build_exception_variant (tree, tree);
extern tree bind_template_template_parm (tree, tree);
extern tree array_type_nelts_total (tree);
extern tree array_type_nelts_top (tree);
-extern tree break_out_target_exprs (tree);
+extern tree break_out_target_exprs (tree, bool = false);
extern tree build_ctor_subob_ref (tree, tree, tree);
extern tree replace_placeholders (tree, tree, bool * = NULL);
extern bool find_placeholders (tree);
diff --git a/gcc/cp/init.c b/gcc/cp/init.c
index e52cd64acc8..5bc8394222b 100644
--- a/gcc/cp/init.c
+++ b/gcc/cp/init.c
@@ -634,7 +634,7 @@ get_nsdmi (tree member, bool in_ctor, tsubst_flags_t complain)
bool simple_target = (init && SIMPLE_TARGET_EXPR_P (init));
if (simple_target)
init = TARGET_EXPR_INITIAL (init);
- init = break_out_target_exprs (init);
+ init = break_out_target_exprs (init, /*loc*/true);
if (simple_target && TREE_CODE (init) != CONSTRUCTOR)
/* Now put it back so C++17 copy elision works. */
init = get_target_expr (init);
diff --git a/gcc/cp/tree.c b/gcc/cp/tree.c
index 18e7793b869..dbe84c08a8f 100644
--- a/gcc/cp/tree.c
+++ b/gcc/cp/tree.c
@@ -2908,12 +2908,19 @@ array_type_nelts_total (tree type)
return sz;
}
+struct bot_data
+{
+ splay_tree target_remap;
+ bool clear_location;
+};
+
/* Called from break_out_target_exprs via mapcar. */
static tree
-bot_manip (tree* tp, int* walk_subtrees, void* data)
+bot_manip (tree* tp, int* walk_subtrees, void* data_)
{
- splay_tree target_remap = ((splay_tree) data);
+ bot_data &data = *(bot_data*)data_;
+ splay_tree target_remap = data.target_remap;
tree t = *tp;
if (!TYPE_P (t) && TREE_CONSTANT (t) && !TREE_SIDE_EFFECTS (t))
@@ -2953,7 +2960,8 @@ bot_manip (tree* tp, int* walk_subtrees, void* data)
(splay_tree_key) TREE_OPERAND (t, 0),
(splay_tree_value) TREE_OPERAND (u, 0));
- TREE_OPERAND (u, 1) = break_out_target_exprs (TREE_OPERAND (u, 1));
+ TREE_OPERAND (u, 1) = break_out_target_exprs (TREE_OPERAND (u, 1),
+ data.clear_location);
if (TREE_OPERAND (u, 1) == error_mark_node)
return error_mark_node;
@@ -2993,6 +3001,8 @@ bot_manip (tree* tp, int* walk_subtrees, void* data)
t = copy_tree_r (tp, walk_subtrees, NULL);
if (TREE_CODE (*tp) == CALL_EXPR)
set_flags_from_callee (*tp);
+ if (data.clear_location && EXPR_HAS_LOCATION (*tp))
+ SET_EXPR_LOCATION (*tp, input_location);
return t;
}
@@ -3001,9 +3011,10 @@ bot_manip (tree* tp, int* walk_subtrees, void* data)
variables. */
static tree
-bot_replace (tree* t, int* /*walk_subtrees*/, void* data)
+bot_replace (tree* t, int* /*walk_subtrees*/, void* data_)
{
- splay_tree target_remap = ((splay_tree) data);
+ bot_data &data = *(bot_data*)data_;
+ splay_tree target_remap = data.target_remap;
if (VAR_P (*t))
{
@@ -3041,10 +3052,13 @@ bot_replace (tree* t, int* /*walk_subtrees*/, void* data)
/* When we parse a default argument expression, we may create
temporary variables via TARGET_EXPRs. When we actually use the
default-argument expression, we make a copy of the expression
- and replace the temporaries with appropriate local versions. */
+ and replace the temporaries with appropriate local versions.
+
+ If CLEAR_LOCATION is true, override any EXPR_LOCATION with
+ input_location. */
tree
-break_out_target_exprs (tree t)
+break_out_target_exprs (tree t, bool clear_location /* = false */)
{
static int target_remap_count;
static splay_tree target_remap;
@@ -3053,9 +3067,10 @@ break_out_target_exprs (tree t)
target_remap = splay_tree_new (splay_tree_compare_pointers,
/*splay_tree_delete_key_fn=*/NULL,
/*splay_tree_delete_value_fn=*/NULL);
- if (cp_walk_tree (&t, bot_manip, target_remap, NULL) == error_mark_node)
+ bot_data data = { target_remap, clear_location };
+ if (cp_walk_tree (&t, bot_manip, &data, NULL) == error_mark_node)
t = error_mark_node;
- cp_walk_tree (&t, bot_replace, target_remap, NULL);
+ cp_walk_tree (&t, bot_replace, &data, NULL);
if (!--target_remap_count)
{
