[PRISM] Fix multiple return with splat and splat+kwsplat

Previously, `return *array, 1` didn't behave like `return [*array, 1]`
properly. Also, it crashed when splat and kwsplat is combined like in
`array = [*things, **hash]`.

Fix this by grouping `PM_ARGUMENTS_NODE` with `PM_ARRAY_NODE` handling and
combining splat and kwsplat handling.
This commit is contained in:
Alan Wu 2024-01-31 16:22:27 -05:00
Родитель 8531ac3115
Коммит 1f226b41f0
2 изменённых файлов: 76 добавлений и 98 удалений

Просмотреть файл

@ -3881,21 +3881,25 @@ pm_compile_node(rb_iseq_t *iseq, const pm_node_t *node, LINK_ANCHOR *const ret,
ADD_LABEL(ret, end_label);
return;
}
case PM_ARGUMENTS_NODE: {
case PM_ARGUMENTS_NODE:
// These are ArgumentsNodes that are not compiled directly by their
// parent call nodes, used in the cases of NextNodes, ReturnNodes
// and BreakNodes
pm_arguments_node_t *arguments_node = (pm_arguments_node_t *) node;
pm_node_list_t node_list = arguments_node->arguments;
for (size_t index = 0; index < node_list.size; index++) {
PM_COMPILE(node_list.nodes[index]);
}
if (node_list.size > 1) {
ADD_INSN1(ret, &dummy_line_node, newarray, INT2FIX(node_list.size));
}
return;
}
// and BreakNodes. They can create an array like ArrayNode.
case PM_ARRAY_NODE: {
const pm_node_list_t *elements;
if (PM_NODE_TYPE(node) == PM_ARGUMENTS_NODE) {
pm_arguments_node_t *cast = (pm_arguments_node_t *)node;
elements = &cast->arguments;
// One element return
if (elements->size == 1) {
PM_COMPILE(elements->nodes[0]);
return;
}
}
else {
pm_array_node_t *cast = (pm_array_node_t *)node;
elements = &cast->elements;
}
// If every node in the array is static, then we can compile the entire
// array now instead of later.
if (pm_static_literal_p(node)) {
@ -3903,8 +3907,7 @@ pm_compile_node(rb_iseq_t *iseq, const pm_node_t *node, LINK_ANCHOR *const ret,
// is popped, then we know we don't need to do anything since it's
// statically known.
if (!popped) {
pm_array_node_t *cast = (pm_array_node_t *) node;
if (cast->elements.size) {
if (elements->size) {
VALUE value = pm_static_literal_value(node, scope_node, parser);
ADD_INSN1(ret, &dummy_line_node, duparray, value);
RB_OBJ_WRITTEN(iseq, Qundef, value);
@ -3913,111 +3916,79 @@ pm_compile_node(rb_iseq_t *iseq, const pm_node_t *node, LINK_ANCHOR *const ret,
ADD_INSN1(ret, &dummy_line_node, newarray, INT2FIX(0));
}
}
} else {
}
else {
// Here since we know there are possible side-effects inside the
// array contents, we're going to build it entirely at runtime.
// We'll do this by pushing all of the elements onto the stack and
// then combining them with newarray.
//
// If this hash is popped, then this serves only to ensure we enact
// If this array is popped, then this serves only to ensure we enact
// all side-effects (like method calls) that are contained within
// the hash contents.
pm_array_node_t *cast = (pm_array_node_t *) node;
pm_node_list_t *elements = &cast->elements;
// the array contents.
// In the case that there is a splat node within the array,
// the array gets compiled slightly differently.
if (node->flags & PM_ARRAY_NODE_FLAGS_CONTAINS_SPLAT) {
if (elements->size == 1) {
// If the only nodes is a SplatNode, we never
// need to emit the newarray or concatarray
// instructions
PM_COMPILE_NOT_POPPED(elements->nodes[0]);
}
else {
// We treat all sequences of non-splat elements as their
// own arrays, followed by a newarray, and then continually
// concat the arrays with the SplatNodes
int new_array_size = 0;
bool need_to_concat_array = false;
for (size_t index = 0; index < elements->size; index++) {
pm_node_t *array_element = elements->nodes[index];
if (PM_NODE_TYPE_P(array_element, PM_SPLAT_NODE)) {
pm_splat_node_t *splat_element = (pm_splat_node_t *)array_element;
// If we already have non-splat elements, we need to emit a newarray
// instruction
if (new_array_size) {
ADD_INSN1(ret, &dummy_line_node, newarray, INT2FIX(new_array_size));
// We don't want to emit a concat array in the case where
// we're seeing our first splat, and already have elements
if (need_to_concat_array) {
ADD_INSN(ret, &dummy_line_node, concatarray);
}
new_array_size = 0;
}
PM_COMPILE_NOT_POPPED(splat_element->expression);
if (index > 0) {
ADD_INSN(ret, &dummy_line_node, concatarray);
}
else {
// If this is the first element, we need to splatarray
ADD_INSN1(ret, &dummy_line_node, splatarray, Qtrue);
}
need_to_concat_array = true;
}
else {
new_array_size++;
PM_COMPILE_NOT_POPPED(array_element);
}
}
// We treat all sequences of non-splat elements as their
// own arrays, followed by a newarray, and then continually
// concat the arrays with the SplatNodes
int new_array_size = 0;
bool need_to_concat_array = false;
bool has_kw_splat = false;
for (size_t index = 0; index < elements->size; index++) {
pm_node_t *array_element = elements->nodes[index];
if (PM_NODE_TYPE_P(array_element, PM_SPLAT_NODE)) {
pm_splat_node_t *splat_element = (pm_splat_node_t *)array_element;
// If we already have non-splat elements, we need to emit a newarray
// instruction
if (new_array_size) {
ADD_INSN1(ret, &dummy_line_node, newarray, INT2FIX(new_array_size));
// We don't want to emit a concat array in the case where
// we're seeing our first splat, and already have elements
if (need_to_concat_array) {
ADD_INSN(ret, &dummy_line_node, concatarray);
}
new_array_size = 0;
}
}
PM_POP_IF_POPPED;
}
else {
bool has_kw_splat = false;
PM_COMPILE_NOT_POPPED(splat_element->expression);
for (size_t index = 0; index < elements->size; index++) {
pm_node_t *element_node = elements->nodes[index];
switch (PM_NODE_TYPE(element_node)) {
case PM_KEYWORD_HASH_NODE: {
has_kw_splat = true;
pm_arg_compile_keyword_hash_node((pm_keyword_hash_node_t *)element_node, iseq, ret, popped, scope_node, dummy_line_node);
break;
}
default: {
PM_COMPILE(element_node);
break;
}
}
}
if (!popped) {
if (has_kw_splat) {
ADD_INSN1(ret, &dummy_line_node, newarraykwsplat, INT2FIX(elements->size));
if (index > 0) {
ADD_INSN(ret, &dummy_line_node, concatarray);
}
else {
ADD_INSN1(ret, &dummy_line_node, newarray, INT2FIX(elements->size));
// If this is the first element, we need to splatarray
ADD_INSN1(ret, &dummy_line_node, splatarray, Qtrue);
}
need_to_concat_array = true;
}
else if (PM_NODE_TYPE_P(array_element, PM_KEYWORD_HASH_NODE)) {
new_array_size++;
has_kw_splat = true;
pm_arg_compile_keyword_hash_node((pm_keyword_hash_node_t *)array_element, iseq, ret, false, scope_node, dummy_line_node);
}
else {
new_array_size++;
PM_COMPILE_NOT_POPPED(array_element);
}
}
if (new_array_size) {
if (has_kw_splat) {
ADD_INSN1(ret, &dummy_line_node, newarraykwsplat, INT2FIX(new_array_size));
}
else {
ADD_INSN1(ret, &dummy_line_node, newarray, INT2FIX(new_array_size));
}
if (need_to_concat_array) {
ADD_INSN(ret, &dummy_line_node, concatarray);
}
}
PM_POP_IF_POPPED;
}
return;
}

Просмотреть файл

@ -1486,6 +1486,13 @@ a
end
prism_test_return_node
CODE
assert_prism_eval(<<-CODE)
def self.prism_test_return_node(*args, **kwargs)
return *args, *args, **kwargs
end
prism_test_return_node(1, foo: 0)
CODE
end
############################################################################