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


> 
> Marek
> 
>

Reply via email to