From 100c20ded12e9112c64b5ba443e19ac82063f887 Mon Sep 17 00:00:00 2001 From: Zakarya Noffel Date: Mon, 26 Oct 2026 22:52:06 +0000 Subject: [PATCH] Unrecognized attribute with a pack expansion in its arguments [GH #412] We call record_pack_expansion_ellipsis_position for unrecognized attributes presumably to silence an error that would occur when we don't know the attribute in a pack expansion e.g. acquire_capability(m...). We keep the arguments as plain text tokens and therefore we don't see it as expanding m (error). So we add a ... when there isn't one and record an expansion and sets the flag. The attempted fix still fakes the ... but only skips setting the flag. The printer stops writing the extra ... but instantiation still repeats or drops the attribute. This change doesn't fake the expansion. If no real ... follows an unrecognized attribute it would just throw away the pack names collected from the attribute's arguments. --- src/attribute.c | 15 +++--- ...h-412-unrecognized-attr-pack-expansion.txt | 20 ++++++++ src/scope_stk.c | 38 +++++++++++++++ src/scope_stk.h | 3 ++ .../edg_x86_32_cp/regressions.log | 2 + tests/expectations/edg_x86_64/regressions.log | 2 + .../edg_x86_64_cp/regressions.log | 2 + .../default.1.1.txt | 1 + .../default.2.1.txt | 1 + .../.test.rto/default.1.1.txt | 34 +++++++++++++ .../.test.rto/default.2.1.txt | 34 +++++++++++++ .../first_stage_il | 4 ++ .../test.mft.cpp | 48 +++++++++++++++++++ ...unrecognized-attr-arg-pack-lengths.sft.cpp | 26 ++++++++++ 14 files changed, 224 insertions(+), 6 deletions(-) create mode 100644 src/pending_changes/gh-412-unrecognized-attr-pack-expansion.txt create mode 100644 tests/tests/regressions/.unrecognized-attr-arg-pack-lengths.rto/default.1.1.txt create mode 100644 tests/tests/regressions/.unrecognized-attr-arg-pack-lengths.rto/default.2.1.txt create mode 100644 tests/tests/regressions/unrecognized-attr-arg-pack-expansion/.test.rto/default.1.1.txt create mode 100644 tests/tests/regressions/unrecognized-attr-arg-pack-expansion/.test.rto/default.2.1.txt create mode 100755 tests/tests/regressions/unrecognized-attr-arg-pack-expansion/first_stage_il create mode 100644 tests/tests/regressions/unrecognized-attr-arg-pack-expansion/test.mft.cpp create mode 100644 tests/tests/regressions/unrecognized-attr-arg-pack-lengths.sft.cpp diff --git a/src/attribute.c b/src/attribute.c index 66a81f25c0..730fef5cf6 100644 --- a/src/attribute.c +++ b/src/attribute.c @@ -2632,12 +2632,15 @@ that appeared in a previous "using" prefix. Can return NULL on error. { a_pack_expansion_descr_ptr pedep; if (pesep != NULL && (*p_attribute == NULL || is_unrecognized_attr(*p_attribute))) { - /* For an unknown attribute, we do not know if packs encountered - within the attribute arguments were expanded within the pack or - should be expanded with the attribute as a whole. By recording - the presence of an ellipsis, we avoid spurious diagnostics. */ - record_pack_expansion_ellipsis_position(&pos_curr_token); - pesep->expansion_with_no_packs_diagnostic_issued = TRUE; + /* An unknown attribute's arguments are raw tokens, in which packs + may already be expanded (e.g., "attr(args...)"). */ + if (curr_token == tok_ellipsis) { + /* A pack expansion, but packs in raw tokens may go undetected. */ + suppress_expansion_with_no_packs_diagnostic(pesep); + } else { + /* Not a pack expansion. */ + discard_pack_references_for_context(pesep); + } /* if */ } /* if */ pedep = end_potential_pack_expansion_context(pesep, /*is_declarator=*/FALSE); diff --git a/src/pending_changes/gh-412-unrecognized-attr-pack-expansion.txt b/src/pending_changes/gh-412-unrecognized-attr-pack-expansion.txt new file mode 100644 index 0000000000..0bda8140cb --- /dev/null +++ b/src/pending_changes/gh-412-unrecognized-attr-pack-expansion.txt @@ -0,0 +1,20 @@ +[GH #412] +Unrecognized attribute with a pack expansion in its arguments + +In a variadic template, an unrecognized attribute whose arguments referenced a +parameter pack (e.g., in a pack expansion) was treated as a pack expansion of +the attribute as a whole. As a result, the C++-generating back end could put +out a spurious "..." after the attribute when generating a template from its +IL, and instantiations could receive the wrong number of copies of the +attribute (e.g., one per pack element, or none for an empty pack). For +example, in Clang mode: + + template struct scoped_lock { + scoped_lock(M&... m) __attribute__((acquire_capability(m...))) {} + }; + +could be generated as + + scoped_lock(M &...m) __attribute((acquire_capability(m ... )...)) { } + +That is now fixed. diff --git a/src/scope_stk.c b/src/scope_stk.c index c4b12ca1a9..0e103d5532 100644 --- a/src/scope_stk.c +++ b/src/scope_stk.c @@ -14232,6 +14232,44 @@ effect and returns NULL. } /* end_potential_pack_expansion_context */ +void discard_pack_references_for_context( + a_pack_expansion_stack_entry_ptr pesep) +/* +This is called just before end_potential_pack_expansion_context for a +potential pack expansion context (described by pesep) that is known not to be +a pack expansion, but that may contain pack references whose expansion cannot +be determined. For example, in + template + void f(M&... m) __attribute__((some_attr(m...))); +the arguments of the unrecognized attribute are recorded as raw tokens, so it +is not known whether m is expanded within them. The pack references recorded +so far for the context are discarded without any diagnostic, so that +end_potential_pack_expansion_context neither records a pack expansion, nor +issues diagnostics for unexpanded packs, nor passes the references on to an +enclosing context. pesep can be NULL, in which case nothing is done; nothing +is done either for suppression, rescan, or real instantiation contexts, in +which pack references are not recorded. +*/ +{ + if (pesep != NULL && !pesep->is_rescan && !pesep->is_suppression && + pesep->instantiation_descr == NULL) { + a_pack_expansion_descr_ptr pedp = pesep->expansion_descr; + check_assertion(pesep == pack_expansion_stack); + /* The context ends here: Extract its pack references and free them. */ + pedp->last_token = curr_token_sequence_number; + extract_pack_references_for_context(pedp); +#if DEBUG + if (db_flag_is_set("packs") && pedp->packs_referenced != NULL) { + fprintf(f_debug, "Discarding the references extracted above\n"); + } /* if */ +#endif /* DEBUG */ + free_list_of_pack_references(pedp->packs_referenced); + pedp->packs_referenced = NULL; + pesep->contains_pack_reference = FALSE; + } /* if */ +} /* discard_pack_references_for_context */ + + a_boolean advance_to_next_pack_element(a_pack_expansion_stack_entry_ptr pesep) /* This routine is called after end_potential_pack_expansion_context has been diff --git a/src/scope_stk.h b/src/scope_stk.h index 1c8607988e..d630c1d6a8 100644 --- a/src/scope_stk.h +++ b/src/scope_stk.h @@ -2667,6 +2667,9 @@ extern a_pack_expansion_descr_ptr end_potential_pack_expansion_context( a_pack_expansion_stack_entry_ptr pesep, a_boolean is_declarator); +extern void discard_pack_references_for_context( + a_pack_expansion_stack_entry_ptr pesep); + extern void begin_tentative_pack_expansion_context(void); extern void end_tentative_pack_expansion_context(a_boolean); diff --git a/tests/expectations/edg_x86_32_cp/regressions.log b/tests/expectations/edg_x86_32_cp/regressions.log index 31f7aba4f2..807d6e936c 100644 --- a/tests/expectations/edg_x86_32_cp/regressions.log +++ b/tests/expectations/edg_x86_32_cp/regressions.log @@ -4,3 +4,5 @@ gh-309.sft.cpp:fp:-DTEST_NUMBER=2 --g++ --no_exceptions:REQUIREMENTS NOT MET: immediate-pragma-lost.sft.cpp:cppbe:-DTEST_NUMBER=1:PASS: pragma-il-class-no-lowering.sft.cpp:fp:-DTEST_NUMBER=1:REQUIREMENTS NOT MET: pragma-il-class.sft.cpp:fp:-DTEST_NUMBER=1:REQUIREMENTS NOT MET: +unrecognized-attr-arg-pack-expansion/test.mft.cpp:fp:-DTEST_NUMBER=1 --c++17 --clang_version 230000:PASS: +unrecognized-attr-arg-pack-expansion/test.mft.cpp:fp:-DTEST_NUMBER=2 --c++17 --gnu_version 150000:PASS: diff --git a/tests/expectations/edg_x86_64/regressions.log b/tests/expectations/edg_x86_64/regressions.log index 541ff91e0e..82e4345b20 100644 --- a/tests/expectations/edg_x86_64/regressions.log +++ b/tests/expectations/edg_x86_64/regressions.log @@ -1,2 +1,4 @@ immediate-pragma-lost.sft.cpp:cppbe:-DTEST_NUMBER=1:REQUIREMENTS NOT MET: modules/crash-processing-imported-explicit-specialization/test.mft.cpp:cp:-DTEST_NUMBER=1:FAIL: +unrecognized-attr-arg-pack-expansion/test.mft.cpp:fp:-DTEST_NUMBER=1 --c++17 --clang_version 230000:REQUIREMENTS NOT MET:Unrecognized attribute with a pack expansion in its arguments +unrecognized-attr-arg-pack-expansion/test.mft.cpp:fp:-DTEST_NUMBER=2 --c++17 --gnu_version 150000:REQUIREMENTS NOT MET:Unrecognized attribute with a pack expansion in its arguments diff --git a/tests/expectations/edg_x86_64_cp/regressions.log b/tests/expectations/edg_x86_64_cp/regressions.log index 31f7aba4f2..807d6e936c 100644 --- a/tests/expectations/edg_x86_64_cp/regressions.log +++ b/tests/expectations/edg_x86_64_cp/regressions.log @@ -4,3 +4,5 @@ gh-309.sft.cpp:fp:-DTEST_NUMBER=2 --g++ --no_exceptions:REQUIREMENTS NOT MET: immediate-pragma-lost.sft.cpp:cppbe:-DTEST_NUMBER=1:PASS: pragma-il-class-no-lowering.sft.cpp:fp:-DTEST_NUMBER=1:REQUIREMENTS NOT MET: pragma-il-class.sft.cpp:fp:-DTEST_NUMBER=1:REQUIREMENTS NOT MET: +unrecognized-attr-arg-pack-expansion/test.mft.cpp:fp:-DTEST_NUMBER=1 --c++17 --clang_version 230000:PASS: +unrecognized-attr-arg-pack-expansion/test.mft.cpp:fp:-DTEST_NUMBER=2 --c++17 --gnu_version 150000:PASS: diff --git a/tests/tests/regressions/.unrecognized-attr-arg-pack-lengths.rto/default.1.1.txt b/tests/tests/regressions/.unrecognized-attr-arg-pack-lengths.rto/default.1.1.txt new file mode 100644 index 0000000000..cd3f642bae --- /dev/null +++ b/tests/tests/regressions/.unrecognized-attr-arg-pack-lengths.rto/default.1.1.txt @@ -0,0 +1 @@ +fe_only -DTEST_NUMBER=1 --c++17 -w --clang_version 230000 Test_name.c diff --git a/tests/tests/regressions/.unrecognized-attr-arg-pack-lengths.rto/default.2.1.txt b/tests/tests/regressions/.unrecognized-attr-arg-pack-lengths.rto/default.2.1.txt new file mode 100644 index 0000000000..9e1b811538 --- /dev/null +++ b/tests/tests/regressions/.unrecognized-attr-arg-pack-lengths.rto/default.2.1.txt @@ -0,0 +1 @@ +fe_only -DTEST_NUMBER=2 --c++17 -w --gnu_version 150000 Test_name.c diff --git a/tests/tests/regressions/unrecognized-attr-arg-pack-expansion/.test.rto/default.1.1.txt b/tests/tests/regressions/unrecognized-attr-arg-pack-expansion/.test.rto/default.1.1.txt new file mode 100644 index 0000000000..6712df9392 --- /dev/null +++ b/tests/tests/regressions/unrecognized-attr-arg-pack-expansion/.test.rto/default.1.1.txt @@ -0,0 +1,34 @@ +./first_stage_il -DTEST_NUMBER=1 --il_display --c++17 --clang_version 230000 Test_name.c +name: file-scope other-text: "acquire_capability" + +name: file-scope other-text: "requires_capability" + +name: file-scope other-text: "acquire_capability" + +name: file-scope other-text: "requires_capability" + +name: file-scope other-text: "acquire_capability" + +name: file-scope other-text: "requires_capability" + +name: file-scope other-text: "release_capability" + +name: file-scope other-text: "try_acquire_capability" + +name: file-scope other-text: "attr_unexpanded" + +is_pack_expansion: TRUE +name: file-scope other-text: "attr_expanded" + +name: file-scope other-text: "release_capability" + +name: file-scope other-text: "try_acquire_capability" + +name: file-scope other-text: "try_acquire_capability" + +name: file-scope other-text: "attr_unexpanded" + +name: file-scope other-text: "attr_expanded" + +name: file-scope other-text: "attr_expanded" + diff --git a/tests/tests/regressions/unrecognized-attr-arg-pack-expansion/.test.rto/default.2.1.txt b/tests/tests/regressions/unrecognized-attr-arg-pack-expansion/.test.rto/default.2.1.txt new file mode 100644 index 0000000000..6ac1ccc383 --- /dev/null +++ b/tests/tests/regressions/unrecognized-attr-arg-pack-expansion/.test.rto/default.2.1.txt @@ -0,0 +1,34 @@ +./first_stage_il -DTEST_NUMBER=2 --il_display --c++17 --gnu_version 150000 Test_name.c +name: file-scope other-text: "acquire_capability" + +name: file-scope other-text: "requires_capability" + +name: file-scope other-text: "acquire_capability" + +name: file-scope other-text: "requires_capability" + +name: file-scope other-text: "acquire_capability" + +name: file-scope other-text: "requires_capability" + +name: file-scope other-text: "release_capability" + +name: file-scope other-text: "try_acquire_capability" + +name: file-scope other-text: "attr_unexpanded" + +is_pack_expansion: TRUE +name: file-scope other-text: "attr_expanded" + +name: file-scope other-text: "release_capability" + +name: file-scope other-text: "try_acquire_capability" + +name: file-scope other-text: "try_acquire_capability" + +name: file-scope other-text: "attr_unexpanded" + +name: file-scope other-text: "attr_expanded" + +name: file-scope other-text: "attr_expanded" + diff --git a/tests/tests/regressions/unrecognized-attr-arg-pack-expansion/first_stage_il b/tests/tests/regressions/unrecognized-attr-arg-pack-expansion/first_stage_il new file mode 100755 index 0000000000..3968f0ad98 --- /dev/null +++ b/tests/tests/regressions/unrecognized-attr-arg-pack-expansion/first_stage_il @@ -0,0 +1,4 @@ +#!/bin/bash +# Direct --il_display to the first (C++-generating) stage of cpfe-cp-gen-be, +# the only one that records unrecognized attributes. +CPFEA_IL_DISPLAY=1 exec $CPFE "$@" diff --git a/tests/tests/regressions/unrecognized-attr-arg-pack-expansion/test.mft.cpp b/tests/tests/regressions/unrecognized-attr-arg-pack-expansion/test.mft.cpp new file mode 100644 index 0000000000..9ecdca44de --- /dev/null +++ b/tests/tests/regressions/unrecognized-attr-arg-pack-expansion/test.mft.cpp @@ -0,0 +1,48 @@ +//type:fp +//options:--c++17 --clang_version 230000:--c++17 --gnu_version 150000 +//options_all:--il_display +//require:RECORD_UNRECOGNIZED_ATTRIBUTES 1 +//script:./first_stage_il +//filter:awk '/^file-scope attribute@/{f=1}/^$/{if (f) print $0; f=0}f' | grep -E -e '^(name|is_pack_expansion):' -e '^$' | sed -e 's/@[0-9a-f]*//' +//remark:Unrecognized attribute with a pack expansion in its arguments + +// An unrecognized attribute whose arguments referenced a pack was treated as +// a pack expansion. Each attribute not followed by "..." must appear once per +// declaration and not be a pack expansion; attr_expanded must remain one. + +struct mutex {}; +struct adopt_lock_t {}; + +template +struct scoped_lock { + scoped_lock(M&... m) __attribute__((acquire_capability(m...))) {} + scoped_lock(adopt_lock_t, M&... m) + __attribute__((requires_capability(m...))) {} + void varargs(int ...); +}; + +template mutex& get(); + +template +void unlock() __attribute__((release_capability(get()...))) {} + +template +void lock(mutex& a, M&... c) + __attribute__((try_acquire_capability(true, a, c...))) {} + +template +void unexpanded(M&... m) __attribute__((attr_unexpanded(m))) {} + +template +[[attr_expanded(T)...]] void expanded() {} + +void f(mutex& a, mutex& b) { + scoped_lock l1(a, b); + scoped_lock l2(adopt_lock_t(), a, b); + scoped_lock<> l3; + unlock<0, 1>(); + lock(a, b); + lock(a); + unexpanded(a, b); + expanded(); +} diff --git a/tests/tests/regressions/unrecognized-attr-arg-pack-lengths.sft.cpp b/tests/tests/regressions/unrecognized-attr-arg-pack-lengths.sft.cpp new file mode 100644 index 0000000000..dab86d449a --- /dev/null +++ b/tests/tests/regressions/unrecognized-attr-arg-pack-lengths.sft.cpp @@ -0,0 +1,26 @@ +//type:fp +//options:--clang_version 230000:--gnu_version 150000 +//options_all:--c++17 -w +//remark:Unrecognized attribute expanding packs of different lengths + +// An unrecognized attribute whose arguments expanded packs of different +// lengths was treated as an expansion of all of them, causing a spurious +// error. Its pack references must also not be diagnosed as unexpanded, nor +// make a later "int ..." look like a parameter pack. + +template struct S { + template struct In { + void f(T&... t, U&... u) __attribute__((requires_capability(t..., u...))); + void g(int ...); + }; +}; +S::In s; + +template struct tl {}; + +template +void h(tl, U... u) __attribute__((unknown_attr(T..., u...))) {} + +void use() { + h(tl(), 1); +}