[clang] [clang] Fix high memory consumption during pack deduction (PR #88637)

via cfe-commits cfe-commits at lists.llvm.org
Sun Apr 14 16:18:42 PDT 2024


================
@@ -831,7 +831,7 @@ class PackDeductionScope {
     if (IsPartiallyExpanded)
       PackElements += NumPartialPackArgs;
     else if (IsExpanded)
-      PackElements += *FixedNumExpansions;
+      PackElements += FixedNumExpansions.value_or(1);
----------------
term-est wrote:

>cannot be the solution. FixedNumExpansions is nullopt. Why is 1 an improvement over nullopt?
`getExpandedPackSize` checks whether the template parameter is a pack, and returns the size of the pack. If the parameter is not a pack, it returns a nullopt. This is why `FixedNumExpansions` is a nullopt, in which case I think `value_or(1)` makes sense as the template parameter is not a pack and it's just a single template parameter. 

A similar approach seems to be used in `getPackIndexForParam` as well
```cpp
    if (PD->isParameterPack()) {
      unsigned NumExpansions =
          S.getNumArgumentsInExpansion(PD->getType(), Args).value_or(1);
 ```

Since `getExpandedPackSize` might return `nullopt`, I assumed the template parameter not being a pack is a valid case and not a bug. Perhaps we can do
```diff
-    if (std::optional<unsigned> ExpandedPackExpansions =
-            getExpandedPackSize(TemplateParams->getParam(Index)))
-      FixedNumExpansions = ExpandedPackExpansions;
+ FixedNumExpansions = getExpandedPackSize(TemplateParams->getParam(Index)).value_or(1);
```
in `addPack` instead to make it more neat? 

I opened this PR thinking that this was a simple issue, but if this is a deeper bug, we can look at -> 

```cpp
    if (Deduced[I].isNull() && Param->isTemplateParameterPack()) {
      if (auto Result =
              PackDeductionScope(S, TemplateParams, Deduced, Info, I).finish();
          Result != TemplateDeductionResult::Success)
        return Result;
    }
 ```
   
Here, `Param->isTemplateParameterPack()` seems to return true, while `getExpandedPackSize` returns `nullopt`

This is because`isTemplateParameterPack` checks whether the given parameter is a `isParameterPack` or not, while `getExpandedPackSize` checks for `isExpandedParameterPack`. In this specific case, former returns true while the latter returns false, so either the parameter is wrongly marked as a parameter pack, or it is not expanded when `PackDeductionScope` is constructed in `ConvertDeducedTemplateArguments'?

If this is indeed a deeper bug than I first thought and `PackDeductionScope` getting constructed with a `Param` that is  not an`isExpandedParameterPack` is an issue, I think there is a missing`TryExpandParameterPacks` call at the TreeTransform stage? I am quite new to codebase so this is just a guess. 

Please give me your opinion regarding this, and I will continue investigating this further depending on that. 

https://github.com/llvm/llvm-project/pull/88637


More information about the cfe-commits mailing list