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); +}