[clang] [Clang] Remove the wrong assumption when rebuilding SizeOfPackExprs for constraint normalization (PR #115120)
Matheus Izvekov via cfe-commits
cfe-commits at lists.llvm.org
Wed Nov 6 12:26:40 PST 2024
================
@@ -1736,23 +1736,13 @@ namespace {
SourceLocation RParenLoc,
std::optional<unsigned> Length,
ArrayRef<TemplateArgument> PartialArgs) {
- if (SemaRef.CodeSynthesisContexts.back().Kind !=
- Sema::CodeSynthesisContext::ConstraintNormalization)
- return inherited::RebuildSizeOfPackExpr(OperatorLoc, Pack, PackLoc,
- RParenLoc, Length, PartialArgs);
-
-#ifndef NDEBUG
- for (auto *Iter = TemplateArgs.begin(); Iter != TemplateArgs.end();
- ++Iter)
- for (const TemplateArgument &TA : Iter->Args)
- assert(TA.getKind() != TemplateArgument::Pack || TA.pack_size() == 1);
-#endif
- Sema::ArgumentPackSubstitutionIndexRAII SubstIndex(
- SemaRef, /*NewSubstitutionIndex=*/0);
- Decl *NewPack = TransformDecl(PackLoc, Pack);
- if (!NewPack)
- return ExprError();
-
+ Decl *NewPack = Pack;
+ if (SemaRef.CodeSynthesisContexts.back().Kind ==
+ Sema::CodeSynthesisContext::ConstraintNormalization) {
+ NewPack = TransformDecl(PackLoc, Pack);
+ if (!NewPack)
+ return ExprError();
+ }
return inherited::RebuildSizeOfPackExpr(OperatorLoc,
cast<NamedDecl>(NewPack), PackLoc,
RParenLoc, Length, PartialArgs);
----------------
mizvekov wrote:
I missed this on the original review, but it's a layering violation to call Transform inside of a Rebuild function.
Can we move this into a Transform* function?
Also, is the `ConstraintNormalization` check necessary for correctness, or is this done for performance reasons?
https://github.com/llvm/llvm-project/pull/115120
More information about the cfe-commits
mailing list