On 11/15/25 6:04 AM, Marek Polacek wrote:
This patch contains the middle-end, libcpp/, c-family/, and cp/ bits
(besides reflect.cc and the gperf bits).
+/* In TREE_VALUE of an attribute this means the attribute argument
+ is never equal to different attribute argument with the same value. */
+#define ATTR_UNIQUE_VALUE_P(NODE) (TREE_LIST_CHECK (NODE)->base.protected_flag)
The word "value" seems superfluous here; it's the attribute that's
unique, not the value. OTOH, I see why it's convenient to put this flag
on the value TREE_LIST rather than the attribute TREE_LIST, but that
means we can only set it for attributes with arguments, and this
directly affects attribute_value_equal, so I guess it's OK. But let's
drop "argument" from the comment. And also mention that the flag
implies order sensitivity, since that doesn't follow from uniqueness.
We need an update to the BINFO_BASE_ACCESSES comment for the additional
information you're adding.
+/* Parse a C++-26 annotation list.
+
+ annotation-list:
+ annotation ... [opt]
+ annotation-list , annotation ... [opt]
+
+ annotation:
+ = constant-expression */
+
+static tree
+cp_parser_annotation_list (cp_parser *parser)
+{
+ tree attributes = NULL_TREE;
+
+ while (true)
+ {
+ cp_token *token = cp_lexer_peek_token (parser->lexer);
+ location_t loc = token->location;
+ if (cp_lexer_next_token_is (parser->lexer, CPP_EQ))
+ {
+ cp_lexer_consume_token (parser->lexer);
+ tree annotation
+ = cp_parser_constant_expression (parser,
+ /*allow_non_constant_p=*/false,
+ /*non_constant_p=*/nullptr,
+ /*strict_p=*/true);
+ if (annotation == error_mark_node)
+ break;
+ auto suppression
+ = make_temp_override (suppress_location_wrappers, 0);
Why here rather than before parsing?
+ annotation = maybe_wrap_with_location (annotation, loc);
...so this wouldn't be needed?
+ annotation = build_tree_list (NULL_TREE, annotation);
+ if (cp_lexer_next_token_is (parser->lexer, CPP_ELLIPSIS))
+ {
+ cp_lexer_consume_token (parser->lexer);
+ annotation = make_pack_expansion (annotation);
+ if (annotation == error_mark_node)
+ break;
+ }
+ attributes = tree_cons (build_tree_list (internal_identifier,
+ annotation_identifier),
+ annotation, attributes);
+ if (processing_template_decl)
+ ATTR_IS_DEPENDENT (attributes) = 1;
This seems redundant with the is_late_template_attribute change.
...> + tree name = get_attribute_name (attr);
+ if (is_attribute_p ("annotation ", name))
This pattern seems common enough to justify adding annotation_p (attr).
@@ -31925,7 +32826,27 @@ cp_parser_std_attribute_list (cp_parser *parser, tree
attr_ns)
while (true)
{
- location_t loc = cp_lexer_peek_token (parser->lexer)->location;
+ token = cp_lexer_peek_token (parser->lexer);
+ location_t loc = token->location;
+ if (token->type == CPP_EQ)
+ {
+ error_at (loc, "mixing annotations and attributes in the same list");
+ cp_lexer_consume_token (parser->lexer);
+ tree annotation
+ = cp_parser_constant_expression (parser,
+ /*allow_non_constant_p=*/false,
+ /*non_constant_p=*/nullptr,
+ /*strict_p=*/true);
+ if (annotation == error_mark_node)
+ break;
+ if (cp_lexer_next_token_is (parser->lexer, CPP_ELLIPSIS))
+ cp_lexer_consume_token (parser->lexer);
+ token = cp_lexer_peek_token (parser->lexer);
+ if (token->type != CPP_COMMA)
+ break;
+ cp_lexer_consume_token (parser->lexer);
+ continue;
+ }
If we're going to do the normal parsing in this erroneous case, let's
factor it out of cp_parser_annotation_list so we can use it here as well.
@@ -12766,7 +12820,11 @@ instantiate_class_template (tree type)
if (base == error_mark_node)
continue;
- base_list = tree_cons (access, base, base_list);
+ tree acc = access;
+ if (annotations)
+ acc = build_tree_list (access, idx == 0 ? annotations
+ : copy_list (annotations));
The copy_list could use a comment; I take it that we want the
annotations for each element of a base-specifier pack-expansion to have
annotations distinct from those of the other elements?
@@ -715,14 +715,20 @@ attribute_takes_identifier_p (const_tree attr_id)
{
const struct attribute_spec *spec = lookup_attribute_spec (attr_id);
if (spec == NULL)
- /* Unknown attribute that we'll end up ignoring, return true so we
- don't complain about an identifier argument. */
- return true;
+ {
+ /* Unknown attribute that we'll end up ignoring, return true so we
+ don't complain about an identifier argument. Except C++
+ annotations. */
+ if (c_dialect_cxx () && id_equal (attr_id, "annotation "))
+ return false;
This seems unreachable, since lookup_attribute_spec should succeed for
annotations?
@@ -1758,10 +1789,25 @@ merge_attributes (tree a1, tree a2)
if (a == NULL_TREE)
{
a1 = copy_node (a2);
- TREE_CHAIN (a1) = attributes;
- attributes = a1;
+ if (a2_unique_value_p)
+ {
+ /* Make sure not to reverse order of a2 chain
+ added before attributes. */
+ *pa = a1;
+ pa = &TREE_CHAIN (a1);
+ }
+ else
+ {
+ TREE_CHAIN (a1) = attributes;
+ attributes = a1;
+ }
}
}
+ if (a2_unique_value_p && a3)
+ {
+ *pa = attributes;
+ attributes = a3;
+ }
Since we need to preserve order in some cases, why not do it in all
cases? The *pa approach doesn't seem any less efficient than the old code.
@@ -957,20 +957,23 @@ decl_attributes (tree *node, tree attributes, int flags,
if (!no_add_attrs)
{
tree old_attrs;
- tree a;
+ tree a = NULL_TREE;
if (DECL_P (*anode))
old_attrs = DECL_ATTRIBUTES (*anode);
else
old_attrs = TYPE_ATTRIBUTES (*anode);
- for (a = find_same_attribute (attr, old_attrs);
- a != NULL_TREE;
- a = find_same_attribute (attr, TREE_CHAIN (a)))
- {
- if (simple_cst_equal (TREE_VALUE (a), args) == 1)
- break;
- }
+ if (!args
+ || TREE_CODE (args) != TREE_LIST
+ || !ATTR_UNIQUE_VALUE_P (args))
+ for (a = find_same_attribute (attr, old_attrs);
+ a != NULL_TREE;
+ a = find_same_attribute (attr, TREE_CHAIN (a)))
+ {
+ if (simple_cst_equal (TREE_VALUE (a), args) == 1)
Pre-existing issue, but it seems like a problem that this doesn't handle
the special cases of attribute_value_equal?
@@ -402,7 +402,7 @@ build_call_a (tree function, int n, tree *argarray)
/* Don't pass empty class objects by value. This is useful
for tags in STL, which are used to control overload resolution.
We don't need to handle other cases of copying empty classes. */
- if (!decl || !fndecl_built_in_p (decl))
+ if (!decl || (!fndecl_built_in_p (decl) && !metafunction_p (decl)))
Why is this needed for metafunctions?
for (i = 0; i < n; i++)
{
tree arg = CALL_EXPR_ARG (function, i);> @@ -7368,6 +7368,13 @@ build_new_op (const op_location_t &loc, enum
tree_code code, int flags,
case LE_EXPR:
case EQ_EXPR:
case NE_EXPR:
+ if (!arg1_type || !arg2_type)
+ {
+ /* Something is very wrong. Perhaps we're trying to compare
+ type nodes. Make sure we've complained. */
+ gcc_assert (seen_error ());
+ return error_mark_node;
+ }
This seems like a temporary workaround that doesn't belong in trunk.
+ /* Some metafunctions aren't dependent just on their arguments, but also
+ on various other dependencies, e.g. has_identifier on a function parameter
+ reflection can change depending on further declarations of corresponding
+ function, is_complete_type depends on type definitions and template
+ specializations in between the calls, define_aggregate even defines
+ class types, etc. Thus, we need to arrange for calls which call
+ at least some metafunctions to be non-cacheable, because their behavior
+ might not be the same. Until we figure out which exact metafunctions
+ need this and which don't, do it for all of them. */
+ bool metafns_called;
Maybe instead we need more calls to clear_cv_and_fold_caches?
@@ -12563,6 +12755,10 @@ potential_constant_expression_1 (tree t, bool
want_rval, bool strict, bool now,
case TU_LOCAL_ENTITY:
return false;
+ /* A splice expression is dependent, so not constant. */
+ case SPLICE_EXPR:
+ return false;
A TEMPLATE_PARM_INDEX is also dependent, but it is potentially-constant
because it will be constant after substitution. The same seems to apply
to splices; I would expect this to return true.
+/* For every DECL_EXPR check if it declares a consteval-only variable and
+ if so, overwrite it with a no-op. The point here is not to leak
+ consteval-only variables into the middle end. */
+
+static tree
+wipe_consteval_only_r (tree *stmt_p, int *, void *)
+{
+ if (TREE_CODE (*stmt_p) == DECL_EXPR)
+ {
+ tree d = DECL_EXPR_DECL (*stmt_p);
+ if (VAR_P (d) && consteval_only_p (d))
+ /* Wipe the DECL_EXPR so that it doesn't get into gimple. */
+ *stmt_p = build1 (NOP_EXPR, void_type_node, integer_zero_node);
Why not void_node? Likewise for the other clobbers in fold_immediate.
+/* Helper macro to set SPLICE_EXPR_EXPRESSION_P. */
+#define SET_SPLICE_EXPR_EXPRESSION_P(NODE) \
+ (SPLICE_EXPR_EXPRESSION_P (TREE_CODE (NODE) == SPLICE_EXPR \
+ ? NODE : TREE_OPERAND (NODE, 0)) = true)
What are these helpers for? What is the node that we expect to have a
splice in op0?
@@ -5514,6 +5582,15 @@ cxx_init_decl_processing (void)
/* Create the `std' namespace. */
push_namespace (get_identifier ("std"));
std_node = current_namespace;
+ if (flag_reflection)
+ {
+ /* Note that we haven't initialized void_type_node yet, so
+ std_meta_node will be initially typeless; its type will be
+ set a little later in init_reflection. */
+ push_namespace (get_identifier ("meta"), /*inline*/false);
+ std_meta_node = current_namespace;
+ pop_namespace ();
+ }
??? std_meta_node is a namespace, which are always typeless.
@@ -9808,6 +9918,20 @@ cp_finish_decl (tree decl, tree init, bool
init_const_expr_p,
record_types_used_by_current_var_decl (decl);
}
+ /* CWG 3115: Every function of consteval-only type shall be an
+ immediate function. */
+ if (TREE_CODE (decl) == FUNCTION_DECL
+ && !DECL_IMMEDIATE_FUNCTION_P (decl)
+ && consteval_only_p (decl)
+ /* But if the function can be escalated, merrily we roll along. */
+ && !immediate_escalating_function_p (decl))
+ {
+ error_at (DECL_SOURCE_LOCATION (decl),
+ "function of consteval-only type must be declared %qs",
+ "consteval");
+ return;
+ }
@@ -20471,6 +20641,16 @@ finish_function (bool inline_p)
unused_but_set_errorcount = errorcount;
}
+ /* CWG 3115: Every function of consteval-only type shall be an immediate
+ function. */
+ if (!DECL_IMMEDIATE_FUNCTION_P (fndecl)
+ && consteval_only_p (fndecl)
+ && !immediate_escalating_function_p (fndecl)
+ && !is_std_allocator_allocate (fndecl))
+ error_at (DECL_SOURCE_LOCATION (fndecl),
+ "function of consteval-only type must be declared %qs",
+ "consteval");
Do we need this in both places?
@@ -2900,6 +2917,70 @@ min_vis_expr_r (tree *tp, int */*walk_subtrees*/, void
*data)
tpvis = type_visibility (DECL_CONTEXT (t));
break;
+ case REFLECT_EXPR:
+ tree r, c;
+ r = REFLECT_EXPR_HANDLE (t);
+ switch (REFLECT_EXPR_KIND (t))
+ {
+ case REFLECT_BASE:
+ /* For direct base class relationship, determine visibility
+ from both D and B types. */
+ tpvis = type_visibility (BINFO_TYPE (r));
+ if (tpvis > *vis_p)
+ *vis_p = tpvis;
+ c = r;
+ while (BINFO_INHERITANCE_CHAIN (c))
Why does this need to loop?
+ c = BINFO_INHERITANCE_CHAIN (c);
+ tpvis = type_visibility (BINFO_TYPE (c));
+ *walk_subtrees = 0;
+ break;
+ case REFLECT_DATA_MEMBER_SPEC:
+ /* For data member description determine visibility
+ from the type. */
+ tpvis = type_visibility (TREE_VEC_ELT (r, 0));
+ *walk_subtrees = 0;
+ break;
+ case REFLECT_PARM:
+ /* For function parameter reflection determine visibility
+ based on parent_of. */
+ tpvis = expr_visibility (DECL_CONTEXT (r));
+ *walk_subtrees = 0;
+ break;
+ case REFLECT_OBJECT:
+ c = get_base_address (r);
+ if ((VAR_P (c) && decl_function_context (c))
+ || TREE_CODE (c) == PARM_DECL)
+ {
+ /* Make reflect_object of block scope variables
+ subobjects local. */
+ tpvis = VISIBILITY_ANON;
+ *walk_subtrees = 0;
+ }
We just stopped treating local declarations as VISIBILITY_ANON in
r16-5024-gf062a6b7985fce -- for this to differ from that, we need more
rationale.
+ break;
+ case REFLECT_ANNOTATION:
+ /* Annotations are always local to the TU. */
+ tpvis = VISIBILITY_ANON;
+ *walk_subtrees = 0;
+ break;
+ default:
+ if (TYPE_P (r))
+ {
+ tpvis = type_visibility (r);
+ *walk_subtrees = 0;
+ break;
+ }
+ if ((VAR_P (r) && decl_function_context (r))
+ || TREE_CODE (r) == PARM_DECL)
+ {
+ /* Block scope variables are local to the TU. */
+ tpvis = VISIBILITY_ANON;
+ *walk_subtrees = 0;
+ break;
+ }
As above, and they should share code, perhaps by the REFLECT_OBJECT case
falling through to the default?
@@ -5130,6 +5219,12 @@ no_linkage_error (tree decl)
/* An imported decl is ok. */
return;
+ /* Metafunctions are magic and should be considered defined even though
+ they have no bodies. ??? This can't be checked in decl_defined_p;
+ we'd get redefinition errors for some of our metafunctions. */
+ if (TREE_CODE (decl) == FUNCTION_DECL && metafunction_p (decl))
+ return;
Why would we complain about metafunctions without this change?
@@ -352,6 +352,10 @@ wrapup_global_declaration_2 (tree decl)
|| (VAR_P (decl) && DECL_HAS_VALUE_EXPR_P (decl)))
return false;
+ /* Compile-only variables are not to be written out. */
+ if (lang_hooks.compile_only_p (decl))
+ return false;
Why don't we just set DECL_EXTERNAL instead of adding a hook?
Not done, but this seems enough for a first pass.
Jason
