c++, v2: Fix up decltype of non-dependent structured binding decl in template [PR92687]
Checks
Commit Message
On Thu, Feb 29, 2024 at 12:50:47PM +0100, Jakub Jelinek wrote:
> finish_decltype_type uses DECL_HAS_VALUE_EXPR_P (expr) check for
> DECL_DECOMPOSITION_P (expr) to determine if it is
> array/struct/vector/complex etc. subobject proxy case vs. structured
> binding using std::tuple_{size,element}.
> For non-templates or when templates are already instantiated, that works
> correctly, finalized DECL_DECOMPOSITION_P non-base vars indeed have
> DECL_VALUE_EXPR in the former case and don't have it in the latter.
> It works fine for dependent structured bindings as well, cp_finish_decomp in
> that case creates DECLTYPE_TYPE tree and defers the handling until
> instantiation.
> As the testcase shows, this doesn't work for the non-dependent structured
> binding case in templates, because DECL_HAS_VALUE_EXPR_P is set in that case
> always; cp_finish_decomp ends with:
> if (processing_template_decl)
> {
> for (unsigned int i = 0; i < count; i++)
> if (!DECL_HAS_VALUE_EXPR_P (v[i]))
> {
> tree a = build_nt (ARRAY_REF, decl, size_int (i),
> NULL_TREE, NULL_TREE);
> SET_DECL_VALUE_EXPR (v[i], a);
> DECL_HAS_VALUE_EXPR_P (v[i]) = 1;
> }
> }
> and those artificial ARRAY_REFs are used in various places during
> instantiation to find out what base the DECL_DECOMPOSITION_P VAR_DECLs
> have and their positions.
> Another option would be to change
> tree
> lookup_decomp_type (tree v)
> {
> - return *decomp_type_table->get (v);
> + if (tree *slot = decomp_type_table->get (v))
> + return *slot;
> + return NULL_TREE;
> }
>
> and in finish_decl_decomp either just in the ptds.saved case or always
> try to lookup_decomp_type, if it returns non-NULL, return what it returned,
> otherwise return unlowered_expr_type (expr). I guess it would be cleaner,
> I thought it would be more costly due to the hash table lookup, but now that
> I think about it again, DECL_VALUE_EXPR is a hash table lookup as well.
> So maybe then
> + if (ptds.saved)
> + {
> + gcc_checking_assert (DECL_HAS_VALUE_EXPR_P (expr));
> + /* DECL_HAS_VALUE_EXPR_P is always set if
> + processing_template_decl. If lookup_decomp_type
> + returns non-NULL, it is the tuple case. */
> + if (tree ret = lookup_decomp_type (expr))
> + return ret;
> + }
> if (DECL_HAS_VALUE_EXPR_P (expr))
> /* Expr is an array or struct subobject proxy, handle
> bit-fields properly. */
> return unlowered_expr_type (expr);
> else
> /* Expr is a reference variable for the tuple case. */
> return lookup_decomp_type (expr);
Here is a variant of the patch which does that.
Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
Or the other version, or adding some flag to the DECL_DECOMPOSITION_P
decls?
2024-03-01 Jakub Jelinek <jakub@redhat.com>
PR c++/92687
* decl.cc (lookup_decomp_type): Return NULL_TREE if decomp_type_table
doesn't have entry for V.
* semantics.cc (finish_decltype_type): If ptds.saved, assert
DECL_HAS_VALUE_EXPR_P is true and decide on tuple vs. non-tuple based
on if lookup_decomp_type is NULL or not.
* g++.dg/cpp1z/decomp59.C: New test.
Jakub
Comments
On 3/1/24 02:55, Jakub Jelinek wrote:
> On Thu, Feb 29, 2024 at 12:50:47PM +0100, Jakub Jelinek wrote:
>> finish_decltype_type uses DECL_HAS_VALUE_EXPR_P (expr) check for
>> DECL_DECOMPOSITION_P (expr) to determine if it is
>> array/struct/vector/complex etc. subobject proxy case vs. structured
>> binding using std::tuple_{size,element}.
>> For non-templates or when templates are already instantiated, that works
>> correctly, finalized DECL_DECOMPOSITION_P non-base vars indeed have
>> DECL_VALUE_EXPR in the former case and don't have it in the latter.
>> It works fine for dependent structured bindings as well, cp_finish_decomp in
>> that case creates DECLTYPE_TYPE tree and defers the handling until
>> instantiation.
>> As the testcase shows, this doesn't work for the non-dependent structured
>> binding case in templates, because DECL_HAS_VALUE_EXPR_P is set in that case
>> always; cp_finish_decomp ends with:
>> if (processing_template_decl)
>> {
>> for (unsigned int i = 0; i < count; i++)
>> if (!DECL_HAS_VALUE_EXPR_P (v[i]))
>> {
>> tree a = build_nt (ARRAY_REF, decl, size_int (i),
>> NULL_TREE, NULL_TREE);
>> SET_DECL_VALUE_EXPR (v[i], a);
>> DECL_HAS_VALUE_EXPR_P (v[i]) = 1;
>> }
>> }
>> and those artificial ARRAY_REFs are used in various places during
>> instantiation to find out what base the DECL_DECOMPOSITION_P VAR_DECLs
>> have and their positions.
>
>> Another option would be to change
>> tree
>> lookup_decomp_type (tree v)
>> {
>> - return *decomp_type_table->get (v);
>> + if (tree *slot = decomp_type_table->get (v))
>> + return *slot;
>> + return NULL_TREE;
>> }
>>
>> and in finish_decl_decomp either just in the ptds.saved case or always
>> try to lookup_decomp_type, if it returns non-NULL, return what it returned,
>> otherwise return unlowered_expr_type (expr). I guess it would be cleaner,
>> I thought it would be more costly due to the hash table lookup, but now that
>> I think about it again, DECL_VALUE_EXPR is a hash table lookup as well.
>> So maybe then
>> + if (ptds.saved)
>> + {
>> + gcc_checking_assert (DECL_HAS_VALUE_EXPR_P (expr));
>> + /* DECL_HAS_VALUE_EXPR_P is always set if
>> + processing_template_decl. If lookup_decomp_type
>> + returns non-NULL, it is the tuple case. */
>> + if (tree ret = lookup_decomp_type (expr))
>> + return ret;
>> + }
>> if (DECL_HAS_VALUE_EXPR_P (expr))
>> /* Expr is an array or struct subobject proxy, handle
>> bit-fields properly. */
>> return unlowered_expr_type (expr);
>> else
>> /* Expr is a reference variable for the tuple case. */
>> return lookup_decomp_type (expr);
>
> Here is a variant of the patch which does that.
>
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
OK.
> Or the other version, or adding some flag to the DECL_DECOMPOSITION_P
> decls?
>
> 2024-03-01 Jakub Jelinek <jakub@redhat.com>
>
> PR c++/92687
> * decl.cc (lookup_decomp_type): Return NULL_TREE if decomp_type_table
> doesn't have entry for V.
> * semantics.cc (finish_decltype_type): If ptds.saved, assert
> DECL_HAS_VALUE_EXPR_P is true and decide on tuple vs. non-tuple based
> on if lookup_decomp_type is NULL or not.
>
> * g++.dg/cpp1z/decomp59.C: New test.
>
> --- gcc/cp/decl.cc.jj 2024-02-28 23:20:01.004751204 +0100
> +++ gcc/cp/decl.cc 2024-02-29 20:03:11.087218176 +0100
> @@ -9262,7 +9262,9 @@ static GTY((cache)) decl_tree_cache_map
> tree
> lookup_decomp_type (tree v)
> {
> - return *decomp_type_table->get (v);
> + if (tree *slot = decomp_type_table->get (v))
> + return *slot;
> + return NULL_TREE;
> }
>
> /* Mangle a decomposition declaration if needed. Arguments like
> --- gcc/cp/semantics.cc.jj 2024-02-28 22:57:08.101800588 +0100
> +++ gcc/cp/semantics.cc 2024-02-29 20:04:51.936880622 +0100
> @@ -11804,6 +11804,15 @@ finish_decltype_type (tree expr, bool id
> access expression). */
> if (DECL_DECOMPOSITION_P (expr))
> {
> + if (ptds.saved)
> + {
> + gcc_checking_assert (DECL_HAS_VALUE_EXPR_P (expr));
> + /* DECL_HAS_VALUE_EXPR_P is always set if
> + processing_template_decl. If lookup_decomp_type
> + returns non-NULL, it is the tuple case. */
> + if (tree ret = lookup_decomp_type (expr))
> + return ret;
> + }
> if (DECL_HAS_VALUE_EXPR_P (expr))
> /* Expr is an array or struct subobject proxy, handle
> bit-fields properly. */
> --- gcc/testsuite/g++.dg/cpp1z/decomp59.C.jj 2024-02-29 20:02:17.467929327 +0100
> +++ gcc/testsuite/g++.dg/cpp1z/decomp59.C 2024-02-29 20:02:17.467929327 +0100
> @@ -0,0 +1,63 @@
> +// PR c++/92687
> +// { dg-do compile { target c++11 } }
> +// { dg-options "" }
> +
> +namespace std {
> + template<typename T> struct tuple_size;
> + template<int, typename> struct tuple_element;
> +}
> +
> +struct A {
> + int i;
> + template <int I> int& get() { return i; }
> +};
> +
> +template<> struct std::tuple_size<A> { static const int value = 2; };
> +template<int I> struct std::tuple_element<I,A> { using type = int; };
> +
> +template<typename T>
> +struct is_reference {
> + static const bool value = false;
> +};
> +
> +template<typename T>
> +struct is_reference<T&>
> +{
> + static const bool value = true;
> +};
> +
> +template<typename T>
> +struct is_reference<T&&>
> +{
> + static const bool value = true;
> +};
> +
> +template<int N>
> +void
> +foo ()
> +{
> + auto [x, y] = A {}; // { dg-warning "structured bindings only available with" "" { target c++14_down } }
> + static_assert (!is_reference<decltype (x)>::value, "");
> +}
> +
> +void
> +bar ()
> +{
> + auto [x, y] = A {}; // { dg-warning "structured bindings only available with" "" { target c++14_down } }
> + static_assert (!is_reference<decltype (x)>::value, "");
> +}
> +
> +template<typename T>
> +void
> +baz ()
> +{
> + auto [x, y] = T {}; // { dg-warning "structured bindings only available with" "" { target c++14_down } }
> + static_assert (!is_reference<decltype (x)>::value, "");
> +}
> +
> +void
> +qux ()
> +{
> + foo<0> ();
> + baz<A> ();
> +}
>
>
> Jakub
>
@@ -9262,7 +9262,9 @@ static GTY((cache)) decl_tree_cache_map
tree
lookup_decomp_type (tree v)
{
- return *decomp_type_table->get (v);
+ if (tree *slot = decomp_type_table->get (v))
+ return *slot;
+ return NULL_TREE;
}
/* Mangle a decomposition declaration if needed. Arguments like
@@ -11804,6 +11804,15 @@ finish_decltype_type (tree expr, bool id
access expression). */
if (DECL_DECOMPOSITION_P (expr))
{
+ if (ptds.saved)
+ {
+ gcc_checking_assert (DECL_HAS_VALUE_EXPR_P (expr));
+ /* DECL_HAS_VALUE_EXPR_P is always set if
+ processing_template_decl. If lookup_decomp_type
+ returns non-NULL, it is the tuple case. */
+ if (tree ret = lookup_decomp_type (expr))
+ return ret;
+ }
if (DECL_HAS_VALUE_EXPR_P (expr))
/* Expr is an array or struct subobject proxy, handle
bit-fields properly. */
@@ -0,0 +1,63 @@
+// PR c++/92687
+// { dg-do compile { target c++11 } }
+// { dg-options "" }
+
+namespace std {
+ template<typename T> struct tuple_size;
+ template<int, typename> struct tuple_element;
+}
+
+struct A {
+ int i;
+ template <int I> int& get() { return i; }
+};
+
+template<> struct std::tuple_size<A> { static const int value = 2; };
+template<int I> struct std::tuple_element<I,A> { using type = int; };
+
+template<typename T>
+struct is_reference {
+ static const bool value = false;
+};
+
+template<typename T>
+struct is_reference<T&>
+{
+ static const bool value = true;
+};
+
+template<typename T>
+struct is_reference<T&&>
+{
+ static const bool value = true;
+};
+
+template<int N>
+void
+foo ()
+{
+ auto [x, y] = A {}; // { dg-warning "structured bindings only available with" "" { target c++14_down } }
+ static_assert (!is_reference<decltype (x)>::value, "");
+}
+
+void
+bar ()
+{
+ auto [x, y] = A {}; // { dg-warning "structured bindings only available with" "" { target c++14_down } }
+ static_assert (!is_reference<decltype (x)>::value, "");
+}
+
+template<typename T>
+void
+baz ()
+{
+ auto [x, y] = T {}; // { dg-warning "structured bindings only available with" "" { target c++14_down } }
+ static_assert (!is_reference<decltype (x)>::value, "");
+}
+
+void
+qux ()
+{
+ foo<0> ();
+ baz<A> ();
+}