[PATCH] D145545: [clang][Interp] Fix local variable (destructor) management in loop bodies

Wed Mar 29 07:07:18 PDT 2023

aaron.ballman accepted this revision.
aaron.ballman added a comment.
This revision is now accepted and ready to land.

LGTM!

================
Comment at: clang/lib/AST/Interp/ByteCodeStmtGen.cpp:324-328
+  LocalScope<Emitter> Scope(this);
+  if (!this->visitUnscopedCompoundStmt(Body))
     return false;
+
+  Scope.emitDestructors();
----------------
tbaeder wrote:
> aaron.ballman wrote:
> > tbaeder wrote:
> > > aaron.ballman wrote:
> > > > `AutoScope` and some curly braces to delimit the scope object lifetime?
> > > The problem is that we want to emit the destructors before the jump, but not destroy the scope. That should happen after the end label, when the loop is finished altogether so an `AutoScope` doesn't work.
> > Ahh yeah, that's a good point... but this still worries me a bit because of how easy it is to misuse the scope after emitting the destructors. Notionally, we want two scopes, right? One scope for the loop construct guts and a second (inner) scope for the loop body. That's how the construct is modeled in the standard: http://eel.is/c++draft/stmt.while#2
> Add a `DestructorScope` that just emits the destructors of a given scope.
Thank you, I like this!

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D145545/new/

https://reviews.llvm.org/D145545