Unrecognized attribute with a pack expansion in its arguments [GH #412]

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

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 <class... M> 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.

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 <class... M>
+  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

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

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:

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

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:

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

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

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"
+

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"
+

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 "$@"

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 <class... M>
+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 <int I> mutex& get();
+
+template <int... I>
+void unlock() __attribute__((release_capability(get<I>()...))) {}
+
+template <class... M>
+void lock(mutex& a, M&... c)
+    __attribute__((try_acquire_capability(true, a, c...))) {}
+
+template <class... M>
+void unexpanded(M&... m) __attribute__((attr_unexpanded(m))) {}
+
+template <class... T>
+[[attr_expanded(T)...]] void expanded() {}
+
+void f(mutex& a, mutex& b) {
+  scoped_lock<mutex, mutex> l1(a, b);
+  scoped_lock<mutex, mutex> l2(adopt_lock_t(), a, b);
+  scoped_lock<> l3;
+  unlock<0, 1>();
+  lock(a, b);
+  lock(a);
+  unexpanded(a, b);
+  expanded<int, char>();
+}

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 <class... T> struct S {
+  template <class... U> struct In {
+    void f(T&... t, U&... u) __attribute__((requires_capability(t..., u...)));
+    void g(int ...);
+  };
+};
+S<int, char>::In<long> s;
+
+template <class... T> struct tl {};
+
+template <class... T, class... U>
+void h(tl<T...>, U... u) __attribute__((unknown_attr(T..., u...))) {}
+
+void use() {
+  h(tl<int, char>(), 1);
+}