This is an archive of the discontinued LLVM Phabricator instance.

[clang][Interp] Make sure we have a variable scope for initializers
ClosedPublic

Authored by tbaeder on Apr 4 2023, 7:38 AM.

Details

Summary
Otherwise, we run into an assertion when trying to use the current
variable scope while creating temporaries for constructor initializers.

Diff Detail

Event Timeline

tbaeder created this revision.Apr 4 2023, 7:38 AM
Herald added a project: Restricted Project. · View Herald TranscriptApr 4 2023, 7:38 AM
tbaeder requested review of this revision.Apr 4 2023, 7:38 AM
Herald added a project: Restricted Project. · View Herald TranscriptApr 4 2023, 7:38 AM
Herald added a subscriber: cfe-commits. · View Herald Transcript
tbaeder updated this revision to Diff 510998.Apr 4 2023, 11:13 PM

Move the scope and add a test that ensures destruction order of the temporaries.

aaron.ballman accepted this revision.Apr 7 2023, 8:19 AM

Generally LGTM but had an additional test request.

clang/test/AST/Interp/records.cpp
313

Would it make sense to give Test a constexpr destructor so that we can validate it's being called? e.g. https://godbolt.org/z/fhE7xzE4e

This revision is now accepted and ready to land.Apr 7 2023, 8:19 AM
tbaeder added inline comments.Apr 8 2023, 12:01 AM
clang/test/AST/Interp/records.cpp
313

That just tests regular destructors, doesn't it? That's already covered by other tests.

aaron.ballman added inline comments.Apr 8 2023, 5:31 AM
clang/test/AST/Interp/records.cpp
313

Yeah, that's fair I suppose. :-)

This revision was landed with ongoing or failed builds.Apr 13 2023, 6:35 AM
This revision was automatically updated to reflect the committed changes.