On 1/17/25 3:08 PM, Jakub Jelinek wrote:
Hi!
This is the first bug discovered today with the
https://gcc.gnu.org/pipermail/gcc-patches/2025-January/673945.html
hack but then turned into proper testcases where embed-21.C FAILed
since introduction of optimized #embed support and the other when
optimizing large C++ initializers using RAW_DATA_CST.
The problem is that the C++ FE calls make_tree_vector_from_ctor
and uses that as arguments vector for deduction guide handling.
The call.cc code isn't prepared to handle RAW_DATA_CST just about
everywhere, so I think it is safer to make sure RAW_DATA_CST only
appears in CONSTRUCTOR_ELTS and nowhere else.
Thus, the following patch expands the RAW_DATA_CSTs from initializers
into multiple INTEGER_CSTs in the returned vector.
Bootstrapped on x86_64-linux and i686-linux, regtests pending, ok
for trunk if it passes?
2025-01-17 Jakub Jelinek <ja...@redhat.com>
PR c++/118528
* c-common.cc (make_tree_vector_from_ctor): Expand RAW_DATA_CST
elements from the CONSTRUCTOR to individual INTEGER_CSTs.
* g++.dg/cpp/embed-21.C: New test.
* g++.dg/cpp2a/class-deduction-aggr16.C: New test.
--- gcc/c-family/c-common.cc.jj 2025-01-02 11:47:29.803228077 +0100
+++ gcc/c-family/c-common.cc 2025-01-17 13:32:53.512294482 +0100
@@ -9016,9 +9016,26 @@ vec<tree, va_gc> *
make_tree_vector_from_ctor (tree ctor)
{
vec<tree,va_gc> *ret = make_tree_vector ();
+ unsigned nelts = CONSTRUCTOR_NELTS (ctor);
vec_safe_reserve (ret, CONSTRUCTOR_NELTS (ctor));
for (unsigned i = 0; i < CONSTRUCTOR_NELTS (ctor); ++i)
- ret->quick_push (CONSTRUCTOR_ELT (ctor, i)->value);
+ if (TREE_CODE (CONSTRUCTOR_ELT (ctor, i)->value) == RAW_DATA_CST)
+ {
+ tree raw_data = CONSTRUCTOR_ELT (ctor, i)->value;
+ nelts += RAW_DATA_LENGTH (raw_data);
+ vec_safe_reserve (ret, nelts);
I notice that the second argument to vec_safe_reserve is the number of
empty spaces to provide, rather than the new desired capacity. So this
will allocate space for an additional copy of the whole constructor.
I guess you want to say reserve (ret, nelts - vec_safe_length (res))?
It's weird that there isn't a more convenient way to express that.
Note that this behavior differs from the C++ standard library, where
reserve indeed specifies the new capacity. This seems likely to cause a
lot of confusion (and over-allocation) going forward. And indeed
looking through exising uses turns up at least two that seem to assume
the standard library semantics (as well as others that assume the actual
semantics).
I wonder about renaming the existing reserve to reserve_space and adding
a reserve_capacity?
Jason
