Skip to content

Commit a4c6d63

Browse files
committed
Apply HINT_BLOCK_SCOPE to lexical variable declarations
At present, any `else {...}` block is wrapped in an ENTER/LEAVE pair, even if no new scope is warranted, which causes runtime inefficiency. For example, this block doesn't need a scope but gets one anyway: ``` } else { return 1; } ``` However, this behaviour means that an object like `$kaboom` will reliably be destroyed when the `else` block is exited: ``` } else { my $kaboom = TickTick->new; } ``` In contrast, `if () {...}` or `elsif () {...}` blocks default to not having an `ENTER/LEAVE` pair, which is more efficient but arguably incorrect, as `$marvin` in this code won't be destroyed when the `if` block is exited: ``` if ( $x ) { my $marvin = TickTick->new; } ``` Exactly when it is destroyed is dependent upon where the next scope exit happens to be, and this could be some ways away from its block. This behaviour is also very brittle, as shown in this case where the no-op `0;` statement causes - via a quirk of parsing - the `if` block to be assigned an `ENTER/LEAVE` pair: ``` if ( $x ) { 0; my $marvin = TickTick->new; } ``` Whether a block gains a scope via `ENTER/LEAVE` pair or is just parented by a `SCOPE` OP (that will be optimized away), is decided by `Perl_op_scope` on the basis of the `OPf_PARENS` flag on a block's `LINESEQ` OP. The parser always sets this flag for `else` blocks, a variety of circumstances determines whether it is set otherwise. This commit makes two changes: * Removes the enforced use of the `OPf_PARENS` flag on `else` blocks. The same rules now apply to `if`, `elsif`, and `else`, hopefully making destruction behaviour more predictable. * Changes the tokenizer such that occurrances of `my`, `our`, and `state` set the `OPf_PARENS` flag, and the containing block will be wrapped in an `ENTER/LEAVE` pair. After this commit, neither of the `if` or `else` block in this example will have an unnecessary`ENTER/LEAVE` pair: ``` if ($x ) { return 0; } else { return 1; } ``` And in this example, both objects will be destroyed when the relevant block is entered: ``` if ( $x ) { my $kaboom_true = TickTick->new; } else { my $kaboom_else = TickTick->new; } ``` Sadly, both blocks in this example will still have unnecessary scopes, but fixing that is for a different PR: ``` if ($x ) { 0; return 0; } else { 0; return 1; } ``` There are two downsides to this commit: 1. It has the potential to change the destruction timing of objects created in an `if` block and assigned to a lexical declared within the block, if the block hasn't ended up with an `ENTER/LEAVE` pair. As noted above though. that behaviour is very brittle and already sensitive to even minor changes to the Perl code - and also to minor parser/optimization changes within the interpreter. 2. The old `else` behaviour meant that the first statement's `NEXTSTATE` OP stays in place and consequently any error messages arising from the first statement mention the correct (or closer) line number. The `if`/`elsif` behaviour is more likely to cause the `NEXTSTATE` OP to be optimized away, causing any error messages to contain the line number for where the `if` or `elsif` keyword keyword occurred, which could be many lines away. Following this commit, first-line error messages will be just as bad for an `else` block as they are for `if`/`elsif` blocks. However, that's a separate issue and should not be a reason to avoid this commit.
1 parent 147d5f1 commit a4c6d63

File tree

5 files changed

+239
-240
lines changed

5 files changed

+239
-240
lines changed

0 commit comments

Comments
 (0)