[clang] Fix typos in member initializers

[kadircet]

Summary

This was regressed in ca619613801233ef2def8c3cc7d311d5ed0033cb. As we
attached InitExprs as-is to the AST, without performing transformations.

[ilya-biryukov]

Randomly chiming in here.
I never had a good model of where CorrectDelayedTyposInExpr, but wanted to note that ActOnFullExpr also calls it. This may be fine, I just wanted to mention it as it stood out.

clang/lib/Sema/SemaDeclCXX.cpp

4101–4105

Should we pass FD here to avoid correcting to itself?

[hokein]

In D142187#4068092, @ilya-biryukov wrote:

Randomly chiming in here.
I never had a good model of where CorrectDelayedTyposInExpr, but wanted to note that ActOnFullExpr also calls it. This may be fine, I just wanted to mention it as it stood out.

+1, we seem to call CorrectDelayedTyposInExpr in an ad-hoc way.

The fix looks good to me, but I will leave the final stamp to @aaron.ballman, @cor3ntin.

clang/test/PCH/typo3.cpp

7

IIUC, the issue is that, we have a dangling TypoExpr under the FieldDecl in the final AST, which violates the contract of TypoExpr.

[aaron.ballman]

clang/lib/Sema/SemaDeclCXX.cpp

4103

Should we be checking for Init.isUsable() before calling .get()?

[kadircet]

clang/lib/Sema/SemaDeclCXX.cpp

4101–4105

unfortunately the interface requires a VarDecl, hence we can't pass a FieldDecl.

4103

because InitExpr is never null here (we bail out early), and because we're converting any leftover TypoExprs to RecoveryExpr Init should always be Usable here. adding an assert.

clang/test/PCH/typo3.cpp

7

yes

[kadircet]

assert on usability of initializer

[aaron.ballman]

LGTM!

[ilya-biryukov]

clang/lib/Sema/SemaDeclCXX.cpp

4101–4105

Ah, it also does not seem to suggest any FieldDecls for typo correction. Nevermind then, sorry for the noise.

clang/lib/Sema/SemaDeclCXX.cpp

void Sema::ActOnFinishCXXInClassMemberInitializer(Decl *D,
   }
 
   if (DiagnoseUnexpandedParameterPack(InitExpr, UPPC_Initializer)) {
     FD->setInvalidDecl();
     FD->removeInClassInitializer();
     return;
   }
 
-  ExprResult Init = InitExpr;
-  if (!FD->getType()->isDependentType() && !InitExpr->isTypeDependent()) {
-    Init = ConvertMemberDefaultInitExpression(FD, InitExpr, InitLoc);
+  ExprResult Init = CorrectDelayedTyposInExpr(InitExpr, /*InitDecl=*/nullptr,
+                                              /*RecoverUncorrectedTypos=*/true);
+  assert(Init.isUsable() && "Init should at least have a RecoveryExpr");
+  if (!FD->getType()->isDependentType() && !Init.get()->isTypeDependent()) {
+    Init = ConvertMemberDefaultInitExpression(FD, Init.get(), InitLoc);
     // C++11 [class.base.init]p7:
     //   The initialization of each base and member constitutes a
     //   full-expression.
     if (!Init.isInvalid())
       Init = ActOnFinishFullExpr(Init.get(), /*DiscarededValue=*/false);
     if (Init.isInvalid()) {
       FD->setInvalidDecl();
       return;
     }
   }
 
-  InitExpr = Init.get();
-
-  FD->setInClassInitializer(InitExpr);
+  FD->setInClassInitializer(Init.get());
 }
 
 /// Find the direct and/or virtual base specifiers that
 /// correspond to the given base type, for use in base initialization
 /// within a constructor.
 static bool FindBaseInitializer(Sema &SemaRef,
                                 CXXRecordDecl *ClassDecl,
                                 QualType BaseType,

clang/test/PCH/typo3.cpp

+// RUN: not %clang_cc1 -emit-pch %s -o %t.pch 2>&1 | FileCheck %s
+
+struct S {
+  // Make sure TypoExprs in default init exprs are corrected before serializing
+  // in PCH.
+  int y = bar;
+  // CHECK: use of undeclared identifier 'bar'
+};