Ping. On Wed, Mar 18, 2026 at 05:59:31PM -0400, Marek Polacek wrote: > On Fri, Mar 06, 2026 at 10:15:37AM +0000, Boris Staletic wrote: > > On Thursday, March 5th, 2026 at 11:31 PM, Marek Polacek > > <[email protected]> wrote: > > > > > On Thu, Mar 05, 2026 at 06:18:13PM -0500, Jason Merrill wrote: > > > > On 3/5/26 5:35 PM, Marek Polacek wrote: > > > > > On Thu, Mar 05, 2026 at 05:13:32PM -0500, Jason Merrill wrote: > > > > > > On 3/5/26 2:07 PM, Marek Polacek wrote: > > > > > > > On Wed, Mar 04, 2026 at 07:13:33PM +0000, Boris Staletic wrote: > > > > > > > > Bootstrapped and tested on x86_64-pc-linux-gnu. > > > > > > > > I have only ran g++.dg/reflect/* tests. > > > > > > > > If needed/wanted, I can run the entire `make check`, though > > > > > > > > that will take me the rest of the night. > > > > > > > > -- >8 -- > > > > > > > > With an annotation like [[=func]] gcc ends up calling > > > > > > > > handle_annotation_attribute with TREE_VALUE (args) being a > > > > > > > > VIEW_CONVERT_EXPR, whose type is a FUNCTION_TYPE. > > > > > > > > That kind of annotation value should be decayed before checking > > > > > > > > if the type is structural. > > > > > > > > > > > > > > > > Signed-off-by: Boris Staletic <[email protected]> > > > > > > > > > > > > > > > > PR c++/124177 > > > > > > > > > > > > > > > > gcc/cp/ChangeLog: > > > > > > > > > > > > > > > > * tree.cc (handle_annotation_attribute): Decay > > > > > > > > annotation > > > > > > > > values. > > > > > > > > > > > > > > This can fit on one line (80 is the limit). > > > > > > > > > > > > I prefer to wrap at 76 in commit messages because git log adds 4 > > > > > > spaces at > > > > > > the beginning. > > > > > > > > > > Fair. > > > > > > > > > > > > > gcc/testsuite/ChangeLog: > > > > > > > > > > > > > > > > * g++.dg/reflect/annotations11.C: New test. > > > > > > > > --- > > > > > > > > gcc/cp/tree.cc | 1 + > > > > > > > > gcc/testsuite/g++.dg/reflect/annotations11.C | 17 > > > > > > > > +++++++++++++++++ > > > > > > > > 2 files changed, 18 insertions(+) > > > > > > > > create mode 100644 > > > > > > > > gcc/testsuite/g++.dg/reflect/annotations11.C > > > > > > > > > > > > > > > > diff --git a/gcc/cp/tree.cc b/gcc/cp/tree.cc > > > > > > > > index 20288ed04e..5c7df0a1d9 100644 > > > > > > > > --- a/gcc/cp/tree.cc > > > > > > > > +++ b/gcc/cp/tree.cc > > > > > > > > @@ -5912,6 +5912,7 @@ handle_annotation_attribute (tree *node, > > > > > > > > tree ARG_UNUSED (name), > > > > > > > > *no_add_attrs = true; > > > > > > > > return NULL_TREE; > > > > > > > > } > > > > > > > > + TREE_VALUE (args) = decay_conversion (TREE_VALUE (args), > > > > > > > > tf_warning_or_error); > > > > > > > > > > > > > > I'm not sure if it's reasonable to call decay_conversion on > > > > > > > something > > > > > > > type-dependent, so perhaps this should be moved inside of the > > > > > > > following > > > > > > > block. > > > > > > > > > > > > Good point. > > > > > > > > > > > > > But annotations are treated as late attributes so I'm not sure > > > > > > > how we'd get here with anything type-dependent, and I couldn't > > > > > > > construct > > > > > > > such a test. Maybe the !t_d_e_p check could be dropped? > > > > > > > > > > > > Or turned into an assert. > > > > > > > > > > Sounds good. I can do it later. > > > > > > > > > > > > We're supposed to treat the expression as reflect_constant(arg) > > > > > > > but > > > > > > > doing the whole convert_reflect_constant_arg thing here seems > > > > > > > unpleasant. > > > > > > > > > > > > Hmm, why unpleasant? It seems straightforward and might be needed > > > > > > to get > > > > > > some messy cases right. > > > > > > > > > > I think it might need > > > > > > > > > > /* Figure out the type for reflect_constant. */ > > > > > type = TREE_TYPE (convert_from_reference (r)); > > > > > type = type_decays_to (type); > > > > > type = cv_unqualified (type); > > > > > > > > convert_for_reference shouldn't be needed here. > > > > > > Without it we don't compile this: > > > > > > constexpr int a = 3; > > > constexpr const int &ref = a; > > > static_assert (type_of (constant_of (^^ref)) == ^^int); > > > > > > due to > > > > > > error: 'const int&' must be a cv-unqualified structural type that is > > > not a reference type > > > > > > > type_decays_to/cv_unqualified to correspond to the [type.deduct.call] > > > > changes in maybe_adjust_types_for_deduction, sure. And then we wouldn't > > > > need the decay_conversion after all. > > > > > > > > > for the type, then check !structural_type_p and copy constructibility, > > > > > then call convert_reflect_constant_arg, and check for error_mark_node. > > > > > > > > Agreed. > > > > > > > > > But I think convert_reflect_constant_arg isn't entirely correct yet > > > > > (FIXME in reflect_constant6.C). > > > > > > > > Perhaps, but that doesn't seem like an argument against using it here. > > > > > > I agree, it will probably work for all our annotation uses. > > > > > > > > > Boris, let me know if you want me to take this over. > > > > I did implement the proposed changes, but it resulted in three regressions. > > Here's the commit with the changes (I was lazy and didn't adjust the commit > > message): > > https://codeberg.org/bstaletic/gcc/commit/8bacdc8ac390df094c9cec74ca6f717168eb7213 > > I pushed my updated commit to my fork[0]. > > > > Make sure to ignore whitespace when looking at the diff, due to indentation > > changes. > > > > The regressions are: > > 1. For `[[=(throw 2, 0)]]`, the annotation evaluation no longer throws. > > Test at: [1] > > 2. One test[2] is now hitting an assert[3] > > 3. Annotations whose value is a user defined type no longer have a proper > > source location. > > 3.1. Existing test case: [4] > > > > For the throw expression, if I change it to `[[=(throw 2)]]`, then I just > > get > > "annotation does not have a structural type", but there's no uncaught > > exception error. > > > > Regarding source location, both DECL_ARTIFICIAL (expr) and DECL_IGNORED_P > > (expr), > > so we end up returning early from maybe_wrap_with_location here: [5] > > I have not checked whether that is also the case for these test cases: > > [6][7] > > > > Then, for the assert, the full trace is: > > > > ``` > > 0x2a9417f internal_error(char const*, ...) > > ../../gcc/diagnostic-global-context.cc:787 > > 0xafd767 fancy_abort(char const*, int, char const*) > > ../../gcc/diagnostics/context.cc:1813 > > 0x830050 create_template_parm_object > > ../../gcc/cp/pt.cc:7504 > > 0x830050 convert_nontype_argument > > ../../gcc/cp/pt.cc:8042 > > 0xd8dc5c convert_reflect_constant_arg(tree_node*, tree_node*) > > ../../gcc/cp/pt.cc:33576 > > 0xe38a34 handle_annotation_attribute > > ../../gcc/cp/tree.cc:5947 > > 0xbf691a xref_basetypes(tree_node*, tree_node*) > > ../../gcc/cp/decl.cc:18611 > > 0xdc5225 instantiate_class_template(tree_node*) > > ../../gcc/cp/pt.cc:12919 > > 0xe3eb03 complete_type(tree_node*) > > ../../gcc/cp/typeck.cc:138 > > 0xe3eca4 complete_type(tree_node*) > > ../../gcc/cp/typeck.cc:164 > > 0xe3eca4 complete_type_or_maybe_complain(tree_node*, tree_node*, int) > > ../../gcc/cp/typeck.cc:151 > > 0xde3a92 eval_bases_of > > ../../gcc/cp/reflect.cc:6845 > > 0xde3a92 process_metafunction(constexpr_ctx const*, tree_node*, tree_node*, > > bool*, bool*, tree_node**) > > ../../gcc/cp/reflect.cc:7661 > > 0xb5c204 cxx_eval_call_expression > > ../../gcc/cp/constexpr.cc:4014 > > 0xb5fcfd cxx_eval_constant_expression(constexpr_ctx const*, tree_node*, > > value_cat, bool*, bool*, tree_node**) > > ../../gcc/cp/constexpr.cc:9352 > > 0xb62b7c cxx_eval_constant_expression(constexpr_ctx const*, tree_node*, > > value_cat, bool*, bool*, tree_node**) > > ../../gcc/cp/constexpr.cc:9489 > > 0xb60d15 cxx_eval_constant_expression(constexpr_ctx const*, tree_node*, > > value_cat, bool*, bool*, tree_node**) > > ../../gcc/cp/constexpr.cc:9770 > > 0xb5c772 cxx_bind_parameters_in_call > > ../../gcc/cp/constexpr.cc:2913 > > 0xb5c772 cxx_eval_call_expression > > ../../gcc/cp/constexpr.cc:4234 > > 0xb5fcfd cxx_eval_constant_expression(constexpr_ctx const*, tree_node*, > > value_cat, bool*, bool*, tree_node**) > > ../../gcc/cp/constexpr.cc:9352 > > ``` > > > > > > > > [0]: > > https://codeberg.org/bstaletic/gcc/commit/8bacdc8ac390df094c9cec74ca6f717168eb7213 > > [1]: > > https://gcc.gnu.org/git/?p=gcc.git;a=blob;f=gcc/testsuite/g%2B%2B.dg/reflect/annotations2.C;h=77b581165f14ddffbba51275caa0b5810d680075;hb=HEAD#l33 > > [2]: > > https://gcc.gnu.org/git/?p=gcc.git;a=blob;f=gcc/testsuite/g%2B%2B.dg/reflect/annotations3.C;h=1f80d7a1851298bc931b89db163711c5a29a3ab9;hb=HEAD#l147 > > [3]: > > https://gcc.gnu.org/git/?p=gcc.git;a=blob;f=gcc/cp/pt.cc;h=ddf492b34354ec1aa398cd945162378a09ac570e;hb=b95955b8854ae1e859cc7c87700922924e9a5e5f#l7504 > > [4]: > > https://gcc.gnu.org/git/?p=gcc.git;a=blob;f=gcc/testsuite/g%2B%2B.dg/reflect/source_location_of1.C;h=8ba34dbbaa7b93cf9d0ae1441581d099c538840e;hb=HEAD#l42 > > [5]: > > https://gcc.gnu.org/git/?p=gcc.git;a=blob;f=gcc/tree.cc;h=d0e745e8d281afce27f8f3f0e25d5b13a5bdc0eb;hb=HEAD#l15000 > > [6]: > > https://gcc.gnu.org/git/?p=gcc.git;a=blob;f=gcc/testsuite/g%2B%2B.dg/reflect/source_location_of1.C;h=8ba34dbbaa7b93cf9d0ae1441581d099c538840e;hb=HEAD#l104 > > [7]: > > https://gcc.gnu.org/git/?p=gcc.git;a=blob;f=gcc/testsuite/g%2B%2B.dg/reflect/source_location_of1.C;h=8ba34dbbaa7b93cf9d0ae1441581d099c538840e;hb=HEAD#l105 > > Uh. I've not been able to make it work with convert_reflect_constant_arg, > either. How about we go back to the original patch, but with the condition > turned into an assert? > > Patch generated with -w. > > Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk? > > -- >8 -- > With an annotation like [[=func]] gcc ends up calling > handle_annotation_attribute with TREE_VALUE (args) being > a VIEW_CONVERT_EXPR, whose type is a FUNCTION_TYPE. That kind of > annotation value should be decayed before checking if the type is > structural. > > PR c++/124177 > > gcc/cp/ChangeLog: > > * tree.cc (handle_annotation_attribute): Decay annotation > values. Turn if !type_dependent_expression_p into an assert. > > gcc/testsuite/ChangeLog: > > * g++.dg/reflect/annotations15.C: New test. > > Co-authored-by: Boris Staletic <[email protected]> > --- > gcc/cp/tree.cc | 9 ++++++--- > gcc/testsuite/g++.dg/reflect/annotations15.C | 17 +++++++++++++++++ > 2 files changed, 23 insertions(+), 3 deletions(-) > create mode 100644 gcc/testsuite/g++.dg/reflect/annotations15.C > > diff --git a/gcc/cp/tree.cc b/gcc/cp/tree.cc > index 7296f2b5c0d..add2758d12f 100644 > --- a/gcc/cp/tree.cc > +++ b/gcc/cp/tree.cc > @@ -5918,8 +5918,12 @@ handle_annotation_attribute (tree *node, tree > ARG_UNUSED (name), > *no_add_attrs = true; > return NULL_TREE; > } > - if (!type_dependent_expression_p (TREE_VALUE (args))) > - { > + > + /* Annotations are treated as late attributes so we shouldn't see > + anything type-dependent now. */ > + gcc_assert (!type_dependent_expression_p (TREE_VALUE (args))); > + TREE_VALUE (args) = decay_conversion (TREE_VALUE (args), > tf_warning_or_error); > + > if (!structural_type_p (TREE_TYPE (TREE_VALUE (args)))) > { > auto_diagnostic_group d; > @@ -5944,7 +5948,6 @@ handle_annotation_attribute (tree *node, tree > ARG_UNUSED (name), > return NULL_TREE; > } > } > - } > if (!processing_template_decl) > { > location_t loc = EXPR_LOCATION (TREE_VALUE (args)); > diff --git a/gcc/testsuite/g++.dg/reflect/annotations15.C > b/gcc/testsuite/g++.dg/reflect/annotations15.C > new file mode 100644 > index 00000000000..8030960b0b2 > --- /dev/null > +++ b/gcc/testsuite/g++.dg/reflect/annotations15.C > @@ -0,0 +1,17 @@ > +// PR c++/124177 > +// { dg-do compile { target c++26 } } > +// { dg-additional-options "-freflection" } > + > +#include <meta> > + > +constexpr int f() { return 42; } > +[[=f]] void g() {} > + > +static_assert (annotations_of (^^g).size () == 1); > +static_assert (extract<int(*)()> (annotations_of (^^g)[0])() == 42); > + > +constexpr int x[5]{}; > +[[=x]] void h() {} > + > +static_assert (annotations_of (^^h).size () == 1); > +static_assert (type_of (annotations_of (^^h)[0]) == ^^const int*); > > base-commit: 5a16651495171d8e6f60df29757f2b396e30a2d5 > -- > 2.53.0 >
Marek
