[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