Skip to content

Commit e6a1baa

Browse files
committed
[PRISM] Fix compiling duplicated keywords
1 parent a5ac27e commit e6a1baa

File tree

1 file changed

+65
-23
lines changed

1 file changed

+65
-23
lines changed

prism_compile.c

Lines changed: 65 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -1117,55 +1117,97 @@ pm_setup_args_core(const pm_arguments_node_t *arguments_node, const pm_node_t *b
11171117
int post_splat_counter = 0;
11181118

11191119
for (size_t index = 0; index < arguments_node_list.size; index++) {
1120-
pm_node_t *argument = arguments_node_list.nodes[index];
1120+
const pm_node_t *argument = arguments_node_list.nodes[index];
11211121

11221122
switch (PM_NODE_TYPE(argument)) {
11231123
// A keyword hash node contains all keyword arguments as AssocNodes and AssocSplatNodes
11241124
case PM_KEYWORD_HASH_NODE: {
1125-
pm_keyword_hash_node_t *keyword_arg = (pm_keyword_hash_node_t *)argument;
1125+
const pm_keyword_hash_node_t *keyword_arg = (const pm_keyword_hash_node_t *) argument;
1126+
const pm_node_list_t *elements = &keyword_arg->elements;
11261127

11271128
if (has_keyword_splat || has_splat) {
11281129
*flags |= VM_CALL_KW_SPLAT;
11291130
has_keyword_splat = true;
1130-
pm_compile_hash_elements(&keyword_arg->elements, nd_line(&dummy_line_node), iseq, ret, scope_node);
1131+
pm_compile_hash_elements(elements, nd_line(&dummy_line_node), iseq, ret, scope_node);
11311132
}
11321133
else {
1133-
size_t len = keyword_arg->elements.size;
1134-
1135-
// We need to first figure out if all elements of the KeywordHashNode are AssocNodes
1136-
// with symbol keys.
1134+
// We need to first figure out if all elements of the
1135+
// KeywordHashNode are AssocNodes with symbol keys.
11371136
if (PM_NODE_FLAG_P(keyword_arg, PM_KEYWORD_HASH_NODE_FLAGS_SYMBOL_KEYS)) {
1138-
// If they are all symbol keys then we can pass them as keyword arguments.
1139-
*kw_arg = rb_xmalloc_mul_add(len, sizeof(VALUE), sizeof(struct rb_callinfo_kwarg));
1137+
// If they are all symbol keys then we can pass them as
1138+
// keyword arguments. The first thing we need to do is
1139+
// deduplicate. We'll do this using the combination of a
1140+
// Ruby hash and a Ruby array.
1141+
VALUE stored_indices = rb_hash_new();
1142+
VALUE keyword_indices = rb_ary_new_capa(elements->size);
1143+
1144+
size_t size = 0;
1145+
for (size_t element_index = 0; element_index < elements->size; element_index++) {
1146+
const pm_assoc_node_t *assoc = (const pm_assoc_node_t *) elements->nodes[element_index];
1147+
1148+
// Retrieve the stored index from the hash for this
1149+
// keyword.
1150+
VALUE keyword = pm_static_literal_value(assoc->key, scope_node);
1151+
VALUE stored_index = rb_hash_aref(stored_indices, keyword);
1152+
1153+
// If this keyword was already seen in the hash,
1154+
// then mark the array at that index as false and
1155+
// decrement the keyword size.
1156+
if (!NIL_P(stored_index)) {
1157+
rb_ary_store(keyword_indices, NUM2LONG(stored_index), Qfalse);
1158+
size--;
1159+
}
1160+
1161+
// Store (and possibly overwrite) the index for this
1162+
// keyword in the hash, mark the array at that index
1163+
// as true, and increment the keyword size.
1164+
rb_hash_aset(stored_indices, keyword, ULONG2NUM(element_index));
1165+
rb_ary_store(keyword_indices, (long) element_index, Qtrue);
1166+
size++;
1167+
}
1168+
1169+
*kw_arg = rb_xmalloc_mul_add(size, sizeof(VALUE), sizeof(struct rb_callinfo_kwarg));
11401170
*flags |= VM_CALL_KWARG;
1171+
11411172
VALUE *keywords = (*kw_arg)->keywords;
11421173
(*kw_arg)->references = 0;
1143-
(*kw_arg)->keyword_len = (int)len;
1174+
(*kw_arg)->keyword_len = (int) size;
11441175

1145-
for (size_t i = 0; i < len; i++) {
1146-
pm_assoc_node_t *assoc = (pm_assoc_node_t *)keyword_arg->elements.nodes[i];
1147-
pm_node_t *key = assoc->key;
1148-
keywords[i] = pm_static_literal_value(key, scope_node);
1149-
PM_COMPILE_NOT_POPPED(assoc->value);
1176+
size_t keyword_index = 0;
1177+
for (size_t element_index = 0; element_index < elements->size; element_index++) {
1178+
const pm_assoc_node_t *assoc = (const pm_assoc_node_t *) elements->nodes[element_index];
1179+
bool popped = true;
1180+
1181+
if (rb_ary_entry(keyword_indices, (long) element_index) == Qtrue) {
1182+
keywords[keyword_index++] = pm_static_literal_value(assoc->key, scope_node);
1183+
popped = false;
1184+
}
1185+
1186+
PM_COMPILE(assoc->value);
11501187
}
1188+
1189+
RUBY_ASSERT(keyword_index == size);
11511190
} else {
1152-
// If they aren't all symbol keys then we need to construct a new hash
1153-
// and pass that as an argument.
1191+
// If they aren't all symbol keys then we need to
1192+
// construct a new hash and pass that as an argument.
11541193
orig_argc++;
11551194
*flags |= VM_CALL_KW_SPLAT;
1156-
if (len > 1) {
1157-
// A new hash will be created for the keyword arguments in this case,
1158-
// so mark the method as passing mutable keyword splat.
1195+
1196+
size_t size = elements->size;
1197+
if (size > 1) {
1198+
// A new hash will be created for the keyword
1199+
// arguments in this case, so mark the method as
1200+
// passing mutable keyword splat.
11591201
*flags |= VM_CALL_KW_SPLAT_MUT;
11601202
}
11611203

1162-
for (size_t i = 0; i < len; i++) {
1163-
pm_assoc_node_t *assoc = (pm_assoc_node_t *)keyword_arg->elements.nodes[i];
1204+
for (size_t element_index = 0; element_index < size; element_index++) {
1205+
pm_assoc_node_t *assoc = (pm_assoc_node_t *) elements->nodes[element_index];
11641206
PM_COMPILE_NOT_POPPED(assoc->key);
11651207
PM_COMPILE_NOT_POPPED(assoc->value);
11661208
}
11671209

1168-
ADD_INSN1(ret, &dummy_line_node, newhash, INT2FIX(len * 2));
1210+
ADD_INSN1(ret, &dummy_line_node, newhash, INT2FIX(size * 2));
11691211
}
11701212
}
11711213
break;

0 commit comments

Comments
 (0)