This is an archive of the discontinued LLVM Phabricator instance.

[Parser] Correct typo after lambda capture initializer is parsed
ClosedPublic

Authored by ahatanak on Oct 3 2016, 12:07 PM.

Details

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

Diff Detail

Event Timeline

ahatanak updated this revision to Diff 73317.Oct 3 2016, 12:07 PM
ahatanak retitled this revision from to [Parser] Correct typo after lambda capture initializer is parsed.
ahatanak updated this object.
ahatanak added a reviewer: rsmith.
ahatanak added a subscriber: cfe-commits.
mehdi_amini added inline comments.
lib/Parse/ParseExprCXX.cpp
951

What happens when there is no typo correction to apply?

ahatanak added inline comments.Oct 21 2016, 5:40 PM
lib/Parse/ParseExprCXX.cpp
951

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

rsmith added inline comments.Oct 25 2016, 3:32 PM
lib/Parse/ParseExprCXX.cpp
951

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 added inline comments.Oct 25 2016, 3:43 PM
lib/Parse/ParseExprCXX.cpp
951

I suggest having a test that exercise this code path.

ahatanak updated this revision to Diff 75819.Oct 25 2016, 5:04 PM
ahatanak marked 2 inline comments as done.

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

Are there any further comments?

arphaman accepted this revision.Dec 19 2016, 3:12 AM
arphaman edited edge metadata.

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.

This revision is now accepted and ready to land.Dec 19 2016, 3:12 AM
This revision was automatically updated to reflect the committed changes.

Thanks all for the review.

cfe/trunk/test/SemaCXX/lambda-expressions.cpp
572 ↗(On Diff #82052)

"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.