This is an archive of the discontinued LLVM Phabricator instance.

Set MustBuildLookupTable on PrimaryContext in ExternalASTMerger
ClosedPublic

Authored by teemperor on Nov 26 2018, 7:28 AM.

Details

Summary

MustBuildLookupTable must always be called on a primary context as we otherwise
trigger an assert, but we don't ensure that this will always happen in our code right now.

This patch explicitly requests the primary context when doing this call as this shouldn't break
anything (as calling getPrimaryContext on a context which is its own primary context is a no-op)
but will catch these rare cases where we somehow operate on a declaration context that is
not its own primary context.

See also D54863.

Diff Detail

Repository
rL LLVM

Event Timeline

teemperor created this revision.Nov 26 2018, 7:28 AM
martong accepted this revision.Nov 26 2018, 8:09 AM

LGTM. Thank you!

This revision is now accepted and ready to land.Nov 26 2018, 8:09 AM

As pointed out in this comment in another review

https://reviews.llvm.org/D44100#1311687

We need to make sure we are running the lldb test suite before committing and watching the bots to make sure the commit does not break them.

Thank you

davide added a subscriber: davide.Nov 28 2018, 11:02 AM

This doesn't break anything, and I'm fairly confident Raphael (@teemperor) ran the tests.

Apologies, I meant to make the comment in the child PR

This revision was automatically updated to reflect the committed changes.