[clang] fix serialization of SubstNonTypeTemplateParmExpr

[mizvekov]

This fixes a couple of mistakes introduced when merging #132748

Fixes msan failure reported here: #132748 (comment)

[llvmbot]

@llvm/pr-subscribers-clang-modules

@llvm/pr-subscribers-clang

Author: Matheus Izvekov (mizvekov)

Changes

This fixes a couple of mistakes introduced when merging #132748

Fixes msan failure reported here: #132748 (comment)

---

Full diff: https://github.com/llvm/llvm-project/pull/134560.diff

3 Files Affected:

• (modified) clang/lib/AST/ASTContext.cpp (+1-1)
• (modified) clang/lib/Serialization/ASTReaderStmt.cpp (+1)
• (modified) clang/lib/Serialization/ASTWriterStmt.cpp (+1)

diff --git a/clang/lib/AST/ASTContext.cpp b/clang/lib/AST/ASTContext.cpp
index 1b6b3d06ddc1e..320fd4e2f3077 100644
--- a/clang/lib/AST/ASTContext.cpp
+++ b/clang/lib/AST/ASTContext.cpp
@@ -7011,7 +7011,7 @@ TemplateName ASTContext::getCanonicalTemplateName(TemplateName Name,
         getCanonicalTemplateArgument(subst->getArgumentPack());
     return getSubstTemplateTemplateParmPack(
         canonArgPack, subst->getAssociatedDecl()->getCanonicalDecl(),
-        subst->getFinal(), subst->getIndex());
+        subst->getIndex(), subst->getFinal());
   }
   case TemplateName::DeducedTemplate: {
     assert(IgnoreDeduced == false);
diff --git a/clang/lib/Serialization/ASTReaderStmt.cpp b/clang/lib/Serialization/ASTReaderStmt.cpp
index d26152f3780ed..22fe54b526433 100644
--- a/clang/lib/Serialization/ASTReaderStmt.cpp
+++ b/clang/lib/Serialization/ASTReaderStmt.cpp
@@ -2229,6 +2229,7 @@ void ASTStmtReader::VisitSubstNonTypeTemplateParmExpr(
     E->PackIndex = Record.readInt();
   else
     E->PackIndex = 0;
+  E->Final = CurrentUnpackingBits->getNextBit();
   E->SubstNonTypeTemplateParmExprBits.NameLoc = readSourceLocation();
   E->Replacement = Record.readSubExpr();
 }
diff --git a/clang/lib/Serialization/ASTWriterStmt.cpp b/clang/lib/Serialization/ASTWriterStmt.cpp
index 23bb5ff22efaf..d0a0f843c7542 100644
--- a/clang/lib/Serialization/ASTWriterStmt.cpp
+++ b/clang/lib/Serialization/ASTWriterStmt.cpp
@@ -2229,6 +2229,7 @@ void ASTStmtWriter::VisitSubstNonTypeTemplateParmExpr(
   CurrentPackingBits.addBit((bool)E->getPackIndex());
   if (auto PackIndex = E->getPackIndex())
     Record.push_back(*PackIndex + 1);
+  CurrentPackingBits.addBit(E->getFinal());
 
   Record.AddSourceLocation(E->getNameLoc());
   Record.AddStmt(E->getReplacement());

[llvm-ci]

LLVM Buildbot has detected a new failure on builder clang-cmake-x86_64-avx512-win running on avx512-intel64-win while building clang at step 4 "cmake stage 1".

Full details are available at: https://lab.llvm.org/buildbot/#/builders/81/builds/6042

Here is the relevant piece of the build log for the reference

Step 4 (cmake stage 1) failure: 'cmake -G ...' (failure)
'cmake' is not recognized as an internal or external command,
operable program or batch file.

[bgra8]

Heads-up @mizvekov we have bisected a non-deterministic compilation down to this change. We're working on a reproducer.

[eaeltsin]

Dumping pcm files that occasionally differ, we always get the same diff:

<     <EXPR_SUBST_NON_TYPE_TEMPLATE_PARM op0=25165824 op1=56 op2=360777253048 op3=3 op4=21560934252/>
---
>     <EXPR_SUBST_NON_TYPE_TEMPLATE_PARM op0=8388608 op1=56 op2=360777253048 op3=3 op4=21560934252/>
66200c66200
<     <EXPR_SUBST_NON_TYPE_TEMPLATE_PARM op0=25165824 op1=56 op2=360777253048 op3=2 op4=21560934252/>
---
>     <EXPR_SUBST_NON_TYPE_TEMPLATE_PARM op0=8388608 op1=56 op2=360777253048 op3=2 op4=21560934252/>
66205c66205
<     <EXPR_SUBST_NON_TYPE_TEMPLATE_PARM op0=25165824 op1=56 op2=360777253048 op3=1 op4=21560934252/>
---
>     <EXPR_SUBST_NON_TYPE_TEMPLATE_PARM op0=8388608 op1=56 op2=360777253048 op3=1 op4=21560934252/>
66223c66223
<     <EXPR_SUBST_NON_TYPE_TEMPLATE_PARM op0=25165824 op1=56 op2=360777253048 op3=3 op4=21560934252/>
---
>     <EXPR_SUBST_NON_TYPE_TEMPLATE_PARM op0=8388608 op1=56 op2=360777253048 op3=3 op4=21560934252/>
66228c66228
<     <EXPR_SUBST_NON_TYPE_TEMPLATE_PARM op0=25165824 op1=56 op2=360777253048 op3=2 op4=21560934252/>
---
>     <EXPR_SUBST_NON_TYPE_TEMPLATE_PARM op0=8388608 op1=56 op2=360777253048 op3=2 op4=21560934252/>
66233c66233
<     <EXPR_SUBST_NON_TYPE_TEMPLATE_PARM op0=25165824 op1=56 op2=360777253048 op3=1 op4=21560934252/>
---
>     <EXPR_SUBST_NON_TYPE_TEMPLATE_PARM op0=8388608 op1=56 op2=360777253048 op3=1 op4=21560934252/>

so op0 non-deterministically changes between 25165824 and 8388608

[eaeltsin]

Okay, this is E->Final, which was

• initialized for VisitSubstNonTypeTemplateParmExpr(SubstNonTypeTemplateParmExpr *E)
• but missed for VisitSubstNonTypeTemplateParmPackExpr(SubstNonTypeTemplateParmPackExpr *E)

@mizvekov - please take a look!

[mizvekov]

Yeah I see, so this was actually missed previously in another node, SubstNonTypeTemplateParmPackExpr, which during substitution propagates this flag to SubstNonTypeTemplateParmExpr.

[eaeltsin]

@mizvekov - can this also be related, or it needs a separate investigation?

[mizvekov]

It's possible but it's not too closely related.

I will post a patch for this one soon, and then we can try again that one and I will take a fresh look on the serialization code.

[mizvekov]

This will be fixed here: #135428