[PATCH] D51597: [ASTImporter] Fix import of VarDecl init
Aleksei Sidorin via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
Sat Sep 15 15:10:17 PDT 2018
a_sidorin accepted this revision.
a_sidorin added inline comments.
This revision is now accepted and ready to land.
================
Comment at: lib/AST/ASTImporter.cpp:1441
+ To->setInit(ToInit);
+ if (From->isInitKnownICE()) {
+ EvaluatedStmt *Eval = To->ensureEvaluatedStmt();
----------------
martong wrote:
> a_sidorin wrote:
> > I see that this is only a code move but I realized that ASTReader and ASTImporter handle this case differently. ASTReader code says:
> >
> > ```
> > if (Val > 1) { // IsInitKnownICE = 1, IsInitNotICE = 2, IsInitICE = 3
> > EvaluatedStmt *Eval = VD->ensureEvaluatedStmt();
> > Eval->CheckedICE = true;
> > Eval->IsICE = Val == 3;
> > }
> > ```
> > but ASTimporter sets these bits only if `isInitKnownICE()` is `true`. This looks strange.
> The comment in ASTReader seems to be wrong and misleading.
> I checked the correspondent code in ASTWriter:
> ```
> Record.push_back(!D->isInitKnownICE() ? 1 : (D->isInitICE() ? 3 : 2));
> ```
> Thus, the comment in ASTReader should be:
> ```
> if (Val > 1) { // IsInitNOTKnownICE = 1, IsInitNotICE = 2, IsInitICE = 3
> ```
> So, `Val > 1` means that the original init expression written by the ASTWriter had the ICE-ness already determined.
> Thus the ASTImporter code seems correct to me.
Thank you for checking this!
The reason I was worrying about this code is that ASTReader/Writer are used in XTU as well so a mismatch between them and ASTImporter can cost us some annoying debug in the future.
Repository:
rC Clang
https://reviews.llvm.org/D51597
More information about the cfe-commits
mailing list