[Parser] Correct typo after lambda capture initializer is parsed

[ahatanak]

Summary

This fixes PR30566.

The assertion is triggered when RecordLayoutBuilder tries to compute the size of a field (for capture "name" in the test case) whose type hasn't been deduced. This fixes the bug by correcting the typo of the capture initializer after the initializer is parsed and before setting the expression for the annotation token.

rdar://problem/23380132

[ahatanak]

ping

[mehdi_amini]

lib/Parse/ParseExprCXX.cpp

951 ↗

(On Diff #73317)

What happens when there is no typo correction to apply?

[ahatanak]

lib/Parse/ParseExprCXX.cpp

951 ↗

(On Diff #73317)

If there are no typos, it just returns the same Expr. If there are typos but no corrections can be applied, it returns ExprError.

[ahatanak]

Add more reviewers.

[rsmith]

lib/Parse/ParseExprCXX.cpp

951 ↗

(On Diff #73317)

If ParseInitializer returned ExprError(), this will incorrectly convert it into ExprResult() (that is, it'll clear the 'invalid' flag). You should skip this step if the initializer expression is not valid.

[mehdi_amini]

lib/Parse/ParseExprCXX.cpp

951 ↗

(On Diff #73317)

I suggest having a test that exercise this code path.

[ahatanak]

Skip the step to correct typo if ParseInitializer returns ExprError(). Add a test case that exercises the change.

[ahatanak]

Are there any further comments?

[arphaman]

This looks good to me. It would be nice if we could get rid of the 2nd expected expression error in auto s0 = S1{[name=]() {}};, but it can be done later.

[ahatanak]

Thanks all for the review.

cfe/trunk/test/SemaCXX/lambda-expressions.cpp

572

"expected expression" error is emitted twice because ParseInitializer() is called twice: first MayBeDesignationStart is called, and then it throws away what was parsed, and then ParseInitializer() is called the second time at Parser::ParseBraceInitializer:427.

I'm not sure what's the best way to fix this.

cfe/trunk/lib/Parse/ParseExprCXX.cpp

if (Tok.is(tok::star)) {
           // FIXME: This is horrible. Adding a mechanism to skip an expression
           // would be much cleaner.
           // FIXME: If there is a ',' before the next ']' or ':', we can skip to
           // that instead. (And if we see a ':' with no matching '?', we can
           // classify this as an Obj-C message send.)
           SourceLocation StartLoc = Tok.getLocation();
           InMessageExpressionRAIIObject MaybeInMessageExpression(*this, true);
           Init = ParseInitializer();
+          if (!Init.isInvalid())
+            Init = Actions.CorrectDelayedTyposInExpr(Init.get());
 
           if (Tok.getLocation() != StartLoc) {
             // Back out the lexing of the token after the initializer.
             PP.RevertCachedTokens(1);
 
             // Replace the consumed tokens with an appropriate annotation.
             Tok.setLocation(StartLoc);
             Tok.setKind(tok::annot_primary_expr);

cfe/trunk/test/SemaCXX/lambda-expressions.cpp

template <class T> struct C {
   };
 };
 
 int func() {
   C<int> a;
   decltype(a)::D b;
 }
 }
+
+namespace PR30566 {
+int name1; // expected-note {{'name1' declared here}}
+
+struct S1 {
+  template<class T>
+  S1(T t) { s = sizeof(t); }
+  int s;
+};
+
+void foo1() {
+  auto s0 = S1{[name=]() {}}; // expected-error 2 {{expected expression}}
+  auto s1 = S1{[name=name]() {}}; // expected-error {{use of undeclared identifier 'name'; did you mean 'name1'?}}
+}
+}