[clang] 7fcba00 - [Sema] Substitute parameter packs when deduced from function arguments (#79371)

via cfe-commits cfe-commits at lists.llvm.org
Sat Jan 27 01:43:42 PST 2024


Author: Gábor Spaits
Date: 2024-01-27T10:43:38+01:00
New Revision: 7fcba000e90b85ce1236a1d43bebb8128332580e

URL: https://github.com/llvm/llvm-project/commit/7fcba000e90b85ce1236a1d43bebb8128332580e
DIFF: https://github.com/llvm/llvm-project/commit/7fcba000e90b85ce1236a1d43bebb8128332580e.diff

LOG: [Sema] Substitute parameter packs when deduced from function arguments (#79371)

This pull request would solve
https://github.com/llvm/llvm-project/issues/78449 .
There is also a discussion about this on stackoverflow:
https://stackoverflow.com/questions/77832658/stdtype-identity-to-support-several-variadic-argument-lists
.

The following program is well formed:
```cpp
#include <type_traits>

template <typename... T>
struct args_tag
{
    using type = std::common_type_t<T...>;
};

template <typename... T>
void bar(args_tag<T...>, std::type_identity_t<T>..., int, std::type_identity_t<T>...) {}

// example
int main() {
    bar(args_tag<int, int>{}, 4, 8, 15, 16, 23);
}
```

but Clang rejects it, while GCC and MSVC doesn't. The reason for this is
that, in `Sema::DeduceTemplateArguments` we are not prepared for this
case.

# Substitution/deduction of parameter packs
The logic that handles substitution when we have explicit template
arguments (`SubstituteExplicitTemplateArguments`) does not work here,
since the types of the pack are not pushed to `ParamTypes` before the
loop starts that does the deduction.
The other "candidate" that may could have handle this case would be the
loop that does the deduction for trailing packs, but we are not dealing
with trailing packs here.

# Solution proposed in this PR
The solution proposed in this PR works similar to the trailing pack
deduction. The main difference here is the end of the deduction cycle.
When a non-trailing template pack argument is found, whose type is not
explicitly specified and the next type is not a pack type, the length of
the previously deduced pack is retrieved (let that length be `s`). After
that the next `s` arguments are processed in the same way as in the case
of non trailing packs.

# Another possible solution
There is another possible approach that would be less efficient. In the
loop when we get to an element of `ParamTypes` that is a pack and could
be substituted because the type is deduced from a previous argument,
then `s` number of arg types would be inserted before the current
element of `ParamTypes` type. Then we would "cancel" the processing of
the current element, first process the previously inserted elements and
the after that re-process the current element.
Basically we would do what `SubstituteExplicitTemplateArguments` does
but during deduction.

# Adjusted test cases
In
`clang/test/CXX/temp/temp.fct.spec/temp.deduct/temp.deduct.call/p1-0x.cpp`
there is a test case named `test_pack_not_at_end` that should work, but
still does not. This test case is relevant because the note for the
error message has changed.
This is what the test case looks like currently:
```cpp
template<typename ...Types>
void pack_not_at_end(tuple<Types...>, Types... values, int); // expected-note {{<int *, double *> vs. <int, int>}}

void test_pack_not_at_end(tuple<int*, double*> t2) {
  pack_not_at_end(t2, 0, 0, 0); // expected-error {{no match}}
  // FIXME: Should the "original argument type must match deduced parameter
  // type" rule apply here?
  pack_not_at_end<int*, double*>(t2, 0, 0, 0); // ok
}

```

The previous note said (before my changes):
```
deduced conflicting types for parameter 'Types' (<int *, double *> vs. <>)
````
The current note says (after my changesand also clang 14 would say this
if the pack was not trailing):
```
deduced conflicting types for parameter 'Types' (<int *, double *> vs. <int, int>)
```
GCC says: 
```
error: no matching function for call to ‘pack_not_at_end(std::tuple<int*, double*>&, int, int, int)’
   70 |     pack_not_at_end(t2, 0, 0, 9); // expected-error {{no match}}
````

---------

Co-authored-by: cor3ntin <corentinjabot at gmail.com>
Co-authored-by: Erich Keane <ekeane at nvidia.com>

Added: 
    

Modified: 
    clang/docs/ReleaseNotes.rst
    clang/lib/Sema/SemaTemplateDeduction.cpp
    clang/test/CXX/temp/temp.fct.spec/temp.deduct/temp.deduct.call/p1-0x.cpp
    clang/test/SemaTemplate/deduction.cpp

Removed: 
    


################################################################################
diff  --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 905a1f3aa3067b..aa06e2b60ce915 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -73,6 +73,9 @@ C++2c Feature Support
 
 Resolutions to C++ Defect Reports
 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+- Substitute template parameter pack, when it is not explicitly specified
+  in the template parameters, but is deduced from a previous argument.
+  (`#78449: <https://github.com/llvm/llvm-project/issues/78449>`_).
 
 C Language Changes
 ------------------

diff  --git a/clang/lib/Sema/SemaTemplateDeduction.cpp b/clang/lib/Sema/SemaTemplateDeduction.cpp
index c9234bf30a5286..25e58f7bdd953d 100644
--- a/clang/lib/Sema/SemaTemplateDeduction.cpp
+++ b/clang/lib/Sema/SemaTemplateDeduction.cpp
@@ -731,6 +731,7 @@ class PackDeductionScope {
   void addPack(unsigned Index) {
     // Save the deduced template argument for the parameter pack expanded
     // by this pack expansion, then clear out the deduction.
+    DeducedFromEarlierParameter = !Deduced[Index].isNull();
     DeducedPack Pack(Index);
     Pack.Saved = Deduced[Index];
     Deduced[Index] = TemplateArgument();
@@ -859,6 +860,23 @@ class PackDeductionScope {
       Info.PendingDeducedPacks[Pack.Index] = Pack.Outer;
   }
 
+  // Return the size of the saved packs if all of them has the same size.
+  std::optional<unsigned> getSavedPackSizeIfAllEqual() const {
+    unsigned PackSize = Packs[0].Saved.pack_size();
+
+    if (std::all_of(Packs.begin() + 1, Packs.end(), [&PackSize](const auto &P) {
+          return P.Saved.pack_size() == PackSize;
+        }))
+      return PackSize;
+    return {};
+  }
+
+  /// Determine whether this pack has already been deduced from a previous
+  /// argument.
+  bool isDeducedFromEarlierParameter() const {
+    return DeducedFromEarlierParameter;
+  }
+
   /// Determine whether this pack has already been partially expanded into a
   /// sequence of (prior) function parameters / template arguments.
   bool isPartiallyExpanded() { return IsPartiallyExpanded; }
@@ -1004,6 +1022,7 @@ class PackDeductionScope {
   unsigned PackElements = 0;
   bool IsPartiallyExpanded = false;
   bool DeducePackIfNotAlreadyDeduced = false;
+  bool DeducedFromEarlierParameter = false;
   /// The number of expansions, if we have a fully-expanded pack in this scope.
   std::optional<unsigned> FixedNumExpansions;
 
@@ -4381,6 +4400,34 @@ Sema::TemplateDeductionResult Sema::DeduceTemplateArguments(
           // corresponding argument is a list?
           PackScope.nextPackElement();
         }
+      } else if (!IsTrailingPack && !PackScope.isPartiallyExpanded() &&
+                 PackScope.isDeducedFromEarlierParameter()) {
+        // [temp.deduct.general#3]
+        // When all template arguments have been deduced
+        // or obtained from default template arguments, all uses of template
+        // parameters in the template parameter list of the template are
+        // replaced with the corresponding deduced or default argument values
+        //
+        // If we have a trailing parameter pack, that has been deduced
+        // previously we substitute the pack here in a similar fashion as
+        // above with the trailing parameter packs. The main 
diff erence here is
+        // that, in this case we are not processing all of the remaining
+        // arguments. We are only process as many arguments as we have in
+        // the already deduced parameter.
+        std::optional<unsigned> ArgPosAfterSubstitution =
+            PackScope.getSavedPackSizeIfAllEqual();
+        if (!ArgPosAfterSubstitution)
+          continue;
+
+        unsigned PackArgEnd = ArgIdx + *ArgPosAfterSubstitution;
+        for (; ArgIdx < PackArgEnd && ArgIdx < Args.size(); ArgIdx++) {
+          ParamTypesForArgChecking.push_back(ParamPattern);
+          if (auto Result = DeduceCallArgument(ParamPattern, ArgIdx,
+                                               /*ExplicitObjetArgument=*/false))
+            return Result;
+
+          PackScope.nextPackElement();
+        }
       }
     }
 

diff  --git a/clang/test/CXX/temp/temp.fct.spec/temp.deduct/temp.deduct.call/p1-0x.cpp b/clang/test/CXX/temp/temp.fct.spec/temp.deduct/temp.deduct.call/p1-0x.cpp
index 1da599ed3c27aa..2ac66b5055ffd5 100644
--- a/clang/test/CXX/temp/temp.fct.spec/temp.deduct/temp.deduct.call/p1-0x.cpp
+++ b/clang/test/CXX/temp/temp.fct.spec/temp.deduct/temp.deduct.call/p1-0x.cpp
@@ -81,7 +81,7 @@ void test_pair_deduction(int *ip, float *fp, double *dp) {
 template<typename ...Types> struct tuple { };
 
 template<typename ...Types>
-void pack_not_at_end(tuple<Types...>, Types... values, int); // expected-note {{<int *, double *> vs. <>}}
+void pack_not_at_end(tuple<Types...>, Types... values, int); // expected-note {{<int *, double *> vs. <int, int>}}
 
 void test_pack_not_at_end(tuple<int*, double*> t2) {
   pack_not_at_end(t2, 0, 0, 0); // expected-error {{no match}}

diff  --git a/clang/test/SemaTemplate/deduction.cpp b/clang/test/SemaTemplate/deduction.cpp
index ad98de61d9e127..e18551bf030222 100644
--- a/clang/test/SemaTemplate/deduction.cpp
+++ b/clang/test/SemaTemplate/deduction.cpp
@@ -13,6 +13,17 @@ struct X0<int, A> {
   static const unsigned value = 1;
 };
 
+template<class T>
+struct type_identity {
+    using type = T;
+};
+
+template<class T>
+using type_identity_t = typename type_identity<T>::type;
+
+template <typename... T>
+struct args_tag {};
+
 template<int> struct X0i;
 template<long> struct X0l;
 int array_x0a[X0<long, X0l>::value == 0? 1 : -1];
@@ -431,6 +442,29 @@ namespace deduction_after_explicit_pack {
     i<int, int>(0, 1, 2, 3, 4, 5); // expected-error {{no match}}
   }
 
+  template <typename... T>
+  void bar(args_tag<T...>, type_identity_t<T>..., int mid, type_identity_t<T>...) {}
+  void call_bar() {
+    bar(args_tag<int, int>{}, 4, 8, 1001, 16, 23);
+  }
+
+  template <typename... Y, typename... T>
+  void foo(args_tag<Y...>, args_tag<T...>, type_identity_t<T>..., int mid, type_identity_t<T>...) {}
+  void call_foo() {
+    foo(args_tag<const int,const int, const int>{}, args_tag<int, int, int>{}, 4, 8, 9, 15, 16, 23, 1);
+  }
+
+  template <typename... Y, typename... T>
+  void foo2(args_tag<Y...>, args_tag<T...>, type_identity_t<T>..., type_identity_t<T>...) {}
+  void call_foo2() {
+    foo2(args_tag<const int,const int, const int>{}, args_tag<int, int, int>{}, 4, 8, 9, 15, 16, 23);
+  }
+
+  template <typename... Y, typename... T> void baz(args_tag<T...>, T..., T...) {}
+  void call_baz() {
+    baz(args_tag<int, int>{}, 1, 2, 3, 4);
+  }
+
   // GCC alarmingly accepts this by deducing T={int} by matching the second
   // parameter against the first argument, then passing the first argument
   // through the first parameter.


        


More information about the cfe-commits mailing list