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.

Author: znoffel1

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