On Fri, Feb 15, 2019 at 01:59:10PM -1000, Jason Merrill wrote:
> On 2/11/19 6:03 PM, Marek Polacek wrote:
> > On Mon, Feb 11, 2019 at 01:43:36PM -0500, Jason Merrill wrote:
> > > On 2/7/19 6:02 PM, Marek Polacek wrote:
> > > > Since r268321 we can call digest_init even in a template, when the
> > > > compound
> > > > literal isn't instantiation-dependent.
> > >
> > > Right. And since digest_init modifies the CONSTRUCTOR in place, that
> > > means
> > > the template trees are digested rather than the original parse trees that
> > > we
> > > try to use. If we're going to use digest_init, we should probably save
> > > another CONSTRUCTOR with the original trees.
> >
> > I tried unsharing the constructor and even its contents but only then did I
> > realize that this cannot work.
>
> Why wouldn't going back to saving {*((struct S *) this)->r} work?
Sorry, I misunderstood what you meant by "saving". I think I do now.
> > It's not digest_init that adds the problematic
> > INDIRECT_REF via convert_from_reference, it's instantiate_pending_templates
> > -> tsubst_expr -> ... -> finish_non_static_data_member.
> >
> > So the problem isn't sharing the contents of the CONSTRUCTOR, but rather
> > what
> > finish_non_static_data_member does with the
> >
> > {.r=(struct R &) (struct R *) ((struct S *) this)->r}
> >
> > expression. The same problem would appear even before r268321 changes if we
> > called tsubst_* twice on the CONSTRUCTOR above.
>
> Yes, it sounds like there's a bug in that path as well. Perhaps
> tsubst_copy_and_build/COMPONENT_REF should strip a REFERENCE_REF_P if t was
> already a reference.
With this patch, this seems no longer to be needed.
> > Do you still think digest_init and/or finish_compound_literal need tweaking?
>
> I imagine that saving post-digest trees might cause other problems, but
> perhaps not. Perhaps we ought to move away more generally from trying to
> save the original parse trees for non-dependent expressions and messing with
> NON_DEPENDENT_EXPR.
Now that I've spent a lot of time looking into 89356 (and the other PRs broken
by the same revision), I'm convinced that we must return the original tree in
finish_compound_literal. But we still have to call digest_init or we lose
detecting narrowing conversions. What happens in that PR is that after
digest_init we lose the braced-init-list and that changes mangling. I've come
up with this fix for 89356, but it also fixes this PR, and likely all the
others.
The comments hopefully explain what I'm doing and why, the only suspicious
thing is the get_target_expr_sfinae call, that is so that initlist109.C
keeps compiling; without the call to get_target_expr_sfinae, we end up
issuing an error in digest_init_r:
1224 if (COMPOUND_LITERAL_P (stripped_init) && code == ARRAY_TYPE)
1225 {
1226 if (complain & tf_error)
1227 error_at (loc, "cannot initialize aggregate of type %qT with "
1228 "a compound literal", type);
But I hope the rest of the patch is reasonable. The LOOKUP_NO_NARROWING bit
isn't necessary but it should be a correct thing to do, so that later in
perform_implicit_conversion_flags we properly set the recently added flag
IMPLICIT_CONV_EXPR_BRACED_INIT.
Bootstrapped/regtested on x86_64-linux and ppc64le-linux, ok for trunk?
2019-02-16 Marek Polacek <pola...@redhat.com>
PR c++/89217 - ICE with list-initialization in range-based for loop.
* constexpr.c (unshare_constructor): No longer static.
* cp-tree.h (unshare_constructor): Declare.
* semantics.c (finish_compound_literal): When dealing with a
non-dependent expression in a template, return the original
expression. Pass LOOKUP_NO_NARROWING to digest_init_flags.
* g++.dg/cpp0x/range-for37.C: New test.
diff --git gcc/cp/constexpr.c gcc/cp/constexpr.c
index 923763faa0a..d946a797999 100644
--- gcc/cp/constexpr.c
+++ gcc/cp/constexpr.c
@@ -1318,7 +1318,7 @@ find_constructor (tree *tp, int *walk_subtrees, void *)
/* If T is a CONSTRUCTOR or an expression that has a CONSTRUCTOR node as a
subexpression, return an unshared copy of T. Otherwise return T. */
-static tree
+tree
unshare_constructor (tree t)
{
tree ctor = walk_tree (&t, find_constructor, NULL, NULL);
diff --git gcc/cp/cp-tree.h gcc/cp/cp-tree.h
index 44a3620a539..60ca1366cf6 100644
--- gcc/cp/cp-tree.h
+++ gcc/cp/cp-tree.h
@@ -7710,6 +7710,7 @@ extern void explain_invalid_constexpr_fn (tree);
extern vec<tree> cx_error_context (void);
extern tree fold_sizeof_expr (tree);
extern void clear_cv_and_fold_caches (void);
+extern tree unshare_constructor (tree);
/* In cp-ubsan.c */
extern void cp_ubsan_maybe_instrument_member_call (tree);
diff --git gcc/cp/semantics.c gcc/cp/semantics.c
index aa5a163dd64..3ecd192bced 100644
--- gcc/cp/semantics.c
+++ gcc/cp/semantics.c
@@ -2796,17 +2796,31 @@ finish_compound_literal (tree type, tree
compound_literal,
return error_mark_node;
}
- if (instantiation_dependent_expression_p (compound_literal)
- || dependent_type_p (type))
+ /* Used to hold a copy of the compound literal in a template. */
+ tree orig_cl = NULL_TREE;
+
+ if (processing_template_decl)
{
- TREE_TYPE (compound_literal) = type;
+ const bool dependent_p
+ = (instantiation_dependent_expression_p (compound_literal)
+ || dependent_type_p (type));
+ if (dependent_p)
+ /* We're about to return, no need to copy. */
+ orig_cl = compound_literal;
+ else
+ /* We're going to need a copy. */
+ orig_cl = unshare_constructor (compound_literal);
+ TREE_TYPE (orig_cl) = type;
/* Mark the expression as a compound literal. */
- TREE_HAS_CONSTRUCTOR (compound_literal) = 1;
+ TREE_HAS_CONSTRUCTOR (orig_cl) = 1;
/* And as instantiation-dependent. */
- CONSTRUCTOR_IS_DEPENDENT (compound_literal) = true;
+ CONSTRUCTOR_IS_DEPENDENT (orig_cl) = dependent_p;
if (fcl_context == fcl_c99)
- CONSTRUCTOR_C99_COMPOUND_LITERAL (compound_literal) = 1;
- return compound_literal;
+ CONSTRUCTOR_C99_COMPOUND_LITERAL (orig_cl) = 1;
+ /* If the compound literal is dependent, we're done for now. */
+ if (dependent_p)
+ return orig_cl;
+ /* Otherwise, do go on to e.g. check narrowing. */
}
type = complete_type (type);
@@ -2842,8 +2856,18 @@ finish_compound_literal (tree type, tree
compound_literal,
if (type == error_mark_node)
return error_mark_node;
}
- compound_literal = digest_init_flags (type, compound_literal, LOOKUP_NORMAL,
+ compound_literal = digest_init_flags (type, compound_literal,
+ LOOKUP_NORMAL | LOOKUP_NO_NARROWING,
complain);
+ /* If we're in a template, return the original compound literal. */
+ if (orig_cl)
+ {
+ if (!VECTOR_TYPE_P (type))
+ return get_target_expr_sfinae (orig_cl, complain);
+ else
+ return orig_cl;
+ }
+
if (TREE_CODE (compound_literal) == CONSTRUCTOR)
{
TREE_HAS_CONSTRUCTOR (compound_literal) = true;
diff --git gcc/testsuite/g++.dg/cpp0x/range-for37.C
gcc/testsuite/g++.dg/cpp0x/range-for37.C
new file mode 100644
index 00000000000..d5c7c091d96
--- /dev/null
+++ gcc/testsuite/g++.dg/cpp0x/range-for37.C
@@ -0,0 +1,24 @@
+// PR c++/89217
+// { dg-do compile { target c++11 } }
+
+struct R {};
+
+struct C
+{
+ R* begin() const { return &r; }
+ R* end() const { return &r; }
+
+ R& r;
+};
+
+struct S
+{
+ void f1() { f2<true>(); }
+ R& r;
+
+ template<bool>
+ void f2()
+ {
+ for (auto i : C{r}) {}
+ }
+};
