[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