This patch fixes 85634, an ice during tsubst where we encounter an
unmarked lookup. Such lookups could be mutated by declarations added
after the template definition containing them. That would be bad.
I had originally wanted to mark these lookups once we knew we were
saving them, but this bug is the final straw in that approach. Instead
I now mark them early when parsing the primary expression they are part
of. At this point we'll have tried tentative parsing, for when they may
have been part of a declaration, and it would be bad to mark them then.
This allows me to rip out all the other places we mark them.
Except for non-dependent ADL lookup. As the comment there says, we end
up saving the overload set and re-jousting them at instantiation time.
Which is unfortunate, but I didn't want to dive into fix that too --
there's an existing bug for that, which my earlier attempts at fixing
broke the concept machinery. So not as simple as one might hope.
The new maybe_fn_decls call is probably useable in a bunch of other
places that call 'is_overloaded_fn .... get_fns'
I'll port a cut down variant of this to the gcc-8 branch.
nathan
--
Nathan Sidwell
2018-06-20 Nathan Sidwell <nat...@acm.org>
PR c++/85634
* cp-tree.h (lookup_keep): Drop KEEP parm.
(lookup_list_keep): Delete.
(maybe_get_fns): Declare.
* parser.c (cp_parser_primary_expression): Call lookup_keep here.
(cp_parser_template_id): Not here ...
* decl.c (cp_finish_decl): ... nor here ...
* init.c (build_raw_new_expr): ... nor here ...
* pt.c (process_template_parm): ... nor here ...
* semantics.c (perform_koenig_lookup): Call lookup_keep.
(finish_call_expr): Not here.
* tree.c (ovl_cache): Delete.
(ovl_make, ovl_copy): No cache.
(lookup_keep): Always keep.
(lookup_list_keep): Delete.
(maybe_get_fns): New, broken out of ...
(get_fns): ... here. Call it.
(built_min_nt_loc, build_min, build_min_non_dep): Drop lookup_keep.
(build_min_nt_call_vec): Likewise.
PR c++/85634
* g++.dg/lookup/pr85634.C: New.
Index: gcc/cp/cp-tree.h
===================================================================
--- gcc/cp/cp-tree.h (revision 261799)
+++ gcc/cp/cp-tree.h (working copy)
@@ -7094,11 +7094,11 @@ extern void lookup_mark (tree lookup,
extern tree lookup_add (tree fns, tree lookup);
extern tree lookup_maybe_add (tree fns, tree lookup,
bool deduping);
-extern void lookup_keep (tree lookup, bool keep);
-extern void lookup_list_keep (tree list, bool keep);
+extern void lookup_keep (tree lookup);
extern int is_overloaded_fn (tree) ATTRIBUTE_PURE;
extern bool really_overloaded_fn (tree) ATTRIBUTE_PURE;
extern tree dependent_name (tree);
+extern tree maybe_get_fns (tree) ATTRIBUTE_PURE;
extern tree get_fns (tree) ATTRIBUTE_PURE;
extern tree get_first_fn (tree) ATTRIBUTE_PURE;
extern tree ovl_scope (tree);
Index: gcc/cp/decl.c
===================================================================
--- gcc/cp/decl.c (revision 261799)
+++ gcc/cp/decl.c (working copy)
@@ -6973,11 +6973,8 @@ cp_finish_decl (tree decl, tree init, bo
}
if (init)
- {
- if (TREE_CODE (init) == TREE_LIST)
- lookup_list_keep (init, true);
- DECL_INITIAL (decl) = init;
- }
+ DECL_INITIAL (decl) = init;
+
if (dep_init)
{
retrofit_lang_decl (decl);
Index: gcc/cp/init.c
===================================================================
--- gcc/cp/init.c (revision 261799)
+++ gcc/cp/init.c (working copy)
@@ -2403,12 +2403,7 @@ build_raw_new_expr (vec<tree, va_gc> *pl
else if (init->is_empty ())
init_list = void_node;
else
- {
- init_list = build_tree_list_vec (init);
- for (tree v = init_list; v; v = TREE_CHAIN (v))
- if (TREE_CODE (TREE_VALUE (v)) == OVERLOAD)
- lookup_keep (TREE_VALUE (v), true);
- }
+ init_list = build_tree_list_vec (init);
new_expr = build4 (NEW_EXPR, build_pointer_type (type),
build_tree_list_vec (placement), type, nelts,
Index: gcc/cp/parser.c
===================================================================
--- gcc/cp/parser.c (revision 261799)
+++ gcc/cp/parser.c (working copy)
@@ -5603,6 +5603,14 @@ cp_parser_primary_expression (cp_parser
}
}
+ if (processing_template_decl)
+ if (tree fns = maybe_get_fns (decl))
+ /* It's too difficult to mark ths in all the places where
+ we know for sure we need to keep the lookup, so do it
+ now. The cost is extra GC to recycle the lookups
+ resolved at parse time. */
+ lookup_keep (fns);
+
decl = (finish_id_expression
(id_expression, decl, parser->scope,
idk,
@@ -15989,11 +15997,6 @@ cp_parser_template_id (cp_parser *parser
token->type = CPP_TEMPLATE_ID;
token->location = combined_loc;
- /* We must mark the lookup as kept, so we don't throw it away on
- the first parse. */
- if (is_overloaded_fn (template_id))
- lookup_keep (get_fns (template_id), true);
-
/* Retrieve any deferred checks. Do not pop this access checks yet
so the memory will not be reclaimed during token replacing below. */
token->u.tree_check_value = ggc_cleared_alloc<struct tree_check> ();
Index: gcc/cp/pt.c
===================================================================
--- gcc/cp/pt.c (revision 261799)
+++ gcc/cp/pt.c (working copy)
@@ -4431,9 +4431,6 @@ process_template_parm (tree list, locati
pushdecl (decl);
- if (defval && TREE_CODE (defval) == OVERLOAD)
- lookup_keep (defval, true);
-
/* Build the parameter node linking the parameter declaration,
its default argument (if any), and its constraints (if any). */
parm = build_tree_list (defval, parm);
Index: gcc/cp/semantics.c
===================================================================
--- gcc/cp/semantics.c (revision 261799)
+++ gcc/cp/semantics.c (working copy)
@@ -2325,6 +2325,11 @@ perform_koenig_lookup (cp_expr fn, vec<t
else
fn = identifier;
}
+ else if (TREE_CODE (fn) == OVERLOAD && processing_template_decl)
+ /* FIXME: We shouldn't really need to mark the lookup here, as
+ resolving the (non-dependent) call should save the single
+ function we resolve to. Related to PR c++/83529. */
+ lookup_keep (fn);
}
if (fn && template_id && fn != error_mark_node)
@@ -2385,10 +2390,7 @@ finish_call_expr (tree fn, vec<tree, va_
SET_EXPR_LOCATION (result, cp_expr_loc_or_loc (fn, input_location));
KOENIG_LOOKUP_P (result) = koenig_p;
if (is_overloaded_fn (fn))
- {
- fn = get_fns (fn);
- lookup_keep (fn, true);
- }
+ fn = get_fns (fn);
if (cfun)
{
@@ -2575,10 +2577,6 @@ finish_call_expr (tree fn, vec<tree, va_
result = convert_from_reference (result);
}
- /* Free or retain OVERLOADs from lookup. */
- if (is_overloaded_fn (orig_fn))
- lookup_keep (get_fns (orig_fn), processing_template_decl);
-
return result;
}
Index: gcc/cp/tree.c
===================================================================
--- gcc/cp/tree.c (revision 261799)
+++ gcc/cp/tree.c (working copy)
@@ -2131,25 +2131,12 @@ build_ref_qualified_type (tree type, cp_
return build_cp_fntype_variant (type, rqual, raises, late);
}
-/* Cache of free ovl nodes. Uses OVL_FUNCTION for chaining. */
-static GTY((deletable)) tree ovl_cache;
-
/* Make a raw overload node containing FN. */
tree
ovl_make (tree fn, tree next)
{
- tree result = ovl_cache;
-
- if (result)
- {
- ovl_cache = OVL_FUNCTION (result);
- /* Zap the flags. */
- memset (result, 0, sizeof (tree_base));
- TREE_SET_CODE (result, OVERLOAD);
- }
- else
- result = make_node (OVERLOAD);
+ tree result = make_node (OVERLOAD);
if (TREE_CODE (fn) == OVERLOAD)
OVL_NESTED_P (result) = true;
@@ -2164,17 +2151,7 @@ ovl_make (tree fn, tree next)
static tree
ovl_copy (tree ovl)
{
- tree result = ovl_cache;
-
- if (result)
- {
- ovl_cache = OVL_FUNCTION (result);
- /* Zap the flags. */
- memset (result, 0, sizeof (tree_base));
- TREE_SET_CODE (result, OVERLOAD);
- }
- else
- result = make_node (OVERLOAD);
+ tree result = make_node (OVERLOAD);
gcc_checking_assert (!OVL_NESTED_P (ovl) && OVL_USED_P (ovl));
TREE_TYPE (result) = TREE_TYPE (ovl);
@@ -2413,44 +2390,22 @@ ovl_used (tree ovl)
}
}
-/* If KEEP is true, preserve the contents of a lookup so that it is
- available for a later instantiation. Otherwise release the LOOKUP
- nodes for reuse. */
+/* Preserve the contents of a lookup so that it is available for a
+ later instantiation. */
void
-lookup_keep (tree lookup, bool keep)
+lookup_keep (tree lookup)
{
for (;
lookup && TREE_CODE (lookup) == OVERLOAD
&& OVL_LOOKUP_P (lookup) && !OVL_USED_P (lookup);
lookup = OVL_CHAIN (lookup))
- if (keep)
- {
- OVL_USED_P (lookup) = true;
- ovl_used (OVL_FUNCTION (lookup));
- }
- else
- {
- OVL_FUNCTION (lookup) = ovl_cache;
- ovl_cache = lookup;
- }
-
- if (keep)
- ovl_used (lookup);
-}
-
-/* LIST is a TREE_LIST whose TREE_VALUEs may be OVERLOADS that need
- keeping, or may be ignored. */
-
-void
-lookup_list_keep (tree list, bool keep)
-{
- for (; list; list = TREE_CHAIN (list))
{
- tree v = TREE_VALUE (list);
- if (TREE_CODE (v) == OVERLOAD)
- lookup_keep (v, keep);
+ OVL_USED_P (lookup) = true;
+ ovl_used (OVL_FUNCTION (lookup));
}
+
+ ovl_used (lookup);
}
/* Returns nonzero if X is an expression for a (possibly overloaded)
@@ -2505,10 +2460,11 @@ really_overloaded_fn (tree x)
return is_overloaded_fn (x) == 2;
}
-/* Get the overload set FROM refers to. */
+/* Get the overload set FROM refers to. Returns NULL if it's not an
+ overload set. */
tree
-get_fns (tree from)
+maybe_get_fns (tree from)
{
/* A baselink is also considered an overloaded function. */
if (TREE_CODE (from) == OFFSET_REF
@@ -2518,9 +2474,23 @@ get_fns (tree from)
from = BASELINK_FUNCTIONS (from);
if (TREE_CODE (from) == TEMPLATE_ID_EXPR)
from = TREE_OPERAND (from, 0);
- gcc_assert (TREE_CODE (from) == OVERLOAD
- || TREE_CODE (from) == FUNCTION_DECL);
- return from;
+
+ if (TREE_CODE (from) == OVERLOAD
+ || TREE_CODE (from) == FUNCTION_DECL)
+ return from;
+
+ return NULL;
+}
+
+/* FROM refers to an overload set. Return that set (or die). */
+
+tree
+get_fns (tree from)
+{
+ tree res = maybe_get_fns (from);
+
+ gcc_assert (res);
+ return res;
}
/* Return the first function of the overload set FROM refers to. */
@@ -3306,12 +3276,7 @@ build_min_nt_loc (location_t loc, enum t
length = TREE_CODE_LENGTH (code);
for (i = 0; i < length; i++)
- {
- tree x = va_arg (p, tree);
- TREE_OPERAND (t, i) = x;
- if (x && TREE_CODE (x) == OVERLOAD)
- lookup_keep (x, true);
- }
+ TREE_OPERAND (t, i) = va_arg (p, tree);
va_end (p);
return t;
@@ -3339,21 +3304,12 @@ build_min (enum tree_code code, tree tt,
{
tree x = va_arg (p, tree);
TREE_OPERAND (t, i) = x;
- if (x)
- {
- if (!TYPE_P (x) && TREE_SIDE_EFFECTS (x))
- TREE_SIDE_EFFECTS (t) = 1;
- if (TREE_CODE (x) == OVERLOAD)
- lookup_keep (x, true);
- }
+ if (x && !TYPE_P (x) && TREE_SIDE_EFFECTS (x))
+ TREE_SIDE_EFFECTS (t) = 1;
}
va_end (p);
- if (code == CAST_EXPR)
- /* The single operand is a TREE_LIST, which we have to check. */
- lookup_list_keep (TREE_OPERAND (t, 0), true);
-
return t;
}
@@ -3382,12 +3338,7 @@ build_min_non_dep (enum tree_code code,
TREE_SIDE_EFFECTS (t) = TREE_SIDE_EFFECTS (non_dep);
for (i = 0; i < length; i++)
- {
- tree x = va_arg (p, tree);
- TREE_OPERAND (t, i) = x;
- if (x && TREE_CODE (x) == OVERLOAD)
- lookup_keep (x, true);
- }
+ TREE_OPERAND (t, i) = va_arg (p, tree);
if (code == COMPOUND_EXPR && TREE_CODE (non_dep) != COMPOUND_EXPR)
/* This should not be considered a COMPOUND_EXPR, because it
@@ -3410,11 +3361,8 @@ build_min_nt_call_vec (tree fn, vec<tree
CALL_EXPR_FN (ret) = fn;
CALL_EXPR_STATIC_CHAIN (ret) = NULL_TREE;
FOR_EACH_VEC_SAFE_ELT (args, ix, t)
- {
- CALL_EXPR_ARG (ret, ix) = t;
- if (TREE_CODE (t) == OVERLOAD)
- lookup_keep (t, true);
- }
+ CALL_EXPR_ARG (ret, ix) = t;
+
return ret;
}
Index: gcc/testsuite/g++.dg/lookup/pr85634.C
===================================================================
--- gcc/testsuite/g++.dg/lookup/pr85634.C (revision 0)
+++ gcc/testsuite/g++.dg/lookup/pr85634.C (working copy)
@@ -0,0 +1,18 @@
+// PR c++/85634 ICE managing lookup set
+
+namespace N {
+ template <class T> void Foo (T *const &);
+}
+
+using namespace N;
+
+template<class T> void Foo (const T &);
+
+
+template <class T>
+void Frob()
+{
+ void (*op)(const T&) = Foo;
+}
+
+template void Frob<int *>();
