I am not an owner of anything in this CL, but I am happy to
offer my thoughts
...
Sure, but Orion is not a committer yet and I need a committer stamp :). Your
thoughts are much appreciated!
https://codereview.chromium.org/1308693014/diff/1/src/interpreter/bytecode-generator.cc
File src/interpreter/bytecode-generator.cc (right):
https://codereview.chromium.org/1308693014/diff/1/src/interpreter/bytecode-generator.cc#newcode50
src/interpreter/bytecode-generator.cc:50: if
(!builder_.HasExplicitReturn()) {
On 2015/09/08 08:25:10, Michael Starzinger wrote:
This looks dangerous. Depending on how control flow is modeled there
could very
well be a case where the last instruction in the byte-code stream is a
return
statement, but it is not guaranteed post-dominate all executions
paths. Consider
the following ...
function f(a) {
if (a) { return 23; }
}
Good point, we need to take this into account, but we would need to take
this into account once we start adding control flow in any case since
the if block will need to jump to an actual bytecode, and the bytecode
array builder will need to back-patch the jump instruction with the
target of the jump.
The last thing the bytecode-generator would do in the graph above would
be to try and back-patch the if jump to target the next bytecode which
is to be emitted. Since that bytecode doesn't exist yet we could easily
spot in the bytecode-array-builder that there is a jump instruction
which is patched to a non-existent bytecode (e.g., just having a field
tracking the greatest bytecode indice which was patched into a jump
instruction and check if that is greater-than/equal than
bytecodes_.size()), and add that to the HasExplicitReturn conditions?
WDYT?
https://codereview.chromium.org/1308693014/
--
--
v8-dev mailing list
v8-dev@googlegroups.com
http://groups.google.com/group/v8-dev
---
You received this message because you are subscribed to the Google Groups "v8-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to v8-dev+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.
