Page MenuHomePhabricator

[Sema] Fix an assert when a block captures a constexpr local
ClosedPublic

Authored by erik.pilkington on Mar 21 2019, 2:31 PM.

Details

Reviewers
rjmccall
rsmith
Summary

MarkVarDeclODRUsed indirectly calls captureInBlock, which creates a copy expression. The copy expression is insulated in it's own ExpressionEvaluationContext, so it saves, mutates, and restores MaybeODRUseExprs as CleanupVarDeclMarking is iterating through it, leading to a crash. Fix this by iterating through a local copy of MaybeODRUseExprs.

rdar://47493525

Diff Detail

Event Timeline

Herald added a project: Restricted Project. · View Herald TranscriptMar 21 2019, 2:31 PM
rjmccall added inline comments.Mar 26 2019, 8:32 AM
clang/lib/Sema/SemaExpr.cpp
15692

It looks like SmallPtrSet's move constructor does actually guarantee to leave the source empty; you could probably just assert that. But I don't think there's an algorithmic cost, so this is fine, too.

Please leave an assertion at the bottom of this function that the set is empty.

erik.pilkington marked an inline comment as done.

Add an assert.

clang/lib/Sema/SemaExpr.cpp
15692

If you have to actually check the constructor to verify the client code is doing the right thing then it makes more sense to just be explicit, IMO. New patch adds the assert though.

rjmccall accepted this revision.Mar 26 2019, 11:28 AM
rjmccall added inline comments.
clang/lib/Sema/SemaExpr.cpp
15692

If it were perf-significant I might disagree, but it shouldn't be, so this is fine. LGTM.

This revision is now accepted and ready to land.Mar 26 2019, 11:28 AM

Landed in r357040. (I forgot to write Differential revision:!)