Skip to content

Commit

Permalink
[ruby/prism] Fix the implementation of the flag on keyword hash nodes
Browse files Browse the repository at this point in the history
The previous implementation was incorrect since it was just checking for all keys in assoc nodes to be static literals but the actual check is that all keys in assoc nodes must be symbol nodes.

This commit fixes that implementation, and, also, aliases the flag to `PM_KEYWORD_HASH_NODE_FLAGS_SYMBOL_KEYS` so that ruby/ruby can start using the new flag name.

I intend to later change the real flag name to `PM_KEYWORD_HASH_NODE_FLAGS_SYMBOL_KEYS` and remove the alias.

ruby/prism@f5099c79ce
  • Loading branch information
paracycle authored and matzbot committed Dec 14, 2023
1 parent c5e3d6d commit 01f21d5
Show file tree
Hide file tree
Showing 11 changed files with 21 additions and 14 deletions.
6 changes: 6 additions & 0 deletions prism/parser.h
Expand Up @@ -17,6 +17,12 @@

#include <stdbool.h>

// TODO: remove this by renaming the original flag
/**
* Temporary alias for the PM_NODE_FLAG_STATIC_KEYS flag.
*/
#define PM_KEYWORD_HASH_NODE_FLAGS_SYMBOL_KEYS PM_KEYWORD_HASH_NODE_FLAGS_STATIC_KEYS

/**
* This enum provides various bits that represent different kinds of states that
* the lexer can track. This is used to determine which kind of token to return
Expand Down
9 changes: 5 additions & 4 deletions prism/prism.c
Expand Up @@ -3949,7 +3949,7 @@ pm_keyword_hash_node_create(pm_parser_t *parser) {
.base = {
.type = PM_KEYWORD_HASH_NODE,
.location = PM_OPTIONAL_LOCATION_NOT_PROVIDED_VALUE,
.flags = PM_KEYWORD_HASH_NODE_FLAGS_STATIC_KEYS
.flags = PM_KEYWORD_HASH_NODE_FLAGS_SYMBOL_KEYS
},
.elements = { 0 }
};
Expand All @@ -3962,10 +3962,11 @@ pm_keyword_hash_node_create(pm_parser_t *parser) {
*/
static void
pm_keyword_hash_node_elements_append(pm_keyword_hash_node_t *hash, pm_node_t *element) {
// If the element being added is not an AssocNode or does not have a static literal key, then
// If the element being added is not an AssocNode or does not have a symbol key, then
// we want to turn the STATIC_KEYS flag off.
if (!PM_NODE_TYPE_P(element, PM_ASSOC_NODE) || !PM_NODE_FLAG_P(((pm_assoc_node_t *) element)->key, PM_NODE_FLAG_STATIC_LITERAL)) {
pm_node_flag_unset((pm_node_t *)hash, PM_KEYWORD_HASH_NODE_FLAGS_STATIC_KEYS);
// TODO: Rename the flag to SYMBOL_KEYS instead.
if (!PM_NODE_TYPE_P(element, PM_ASSOC_NODE) || !PM_NODE_TYPE_P(((pm_assoc_node_t *) element)->key, PM_SYMBOL_NODE)) {
pm_node_flag_unset((pm_node_t *)hash, PM_KEYWORD_HASH_NODE_FLAGS_SYMBOL_KEYS);
}

pm_node_list_append(&hash->elements, element);
Expand Down
2 changes: 1 addition & 1 deletion test/prism/snapshots/seattlerb/aref_args_assocs.txt
Expand Up @@ -7,7 +7,7 @@
├── flags: ∅
├── elements: (length: 1)
│ └── @ KeywordHashNode (location: (1,1)-(1,7))
│ ├── flags: static_keys
│ ├── flags:
│ └── elements: (length: 1)
│ └── @ AssocNode (location: (1,1)-(1,7))
│ ├── key:
Expand Down
2 changes: 1 addition & 1 deletion test/prism/snapshots/seattlerb/aref_args_lit_assocs.txt
Expand Up @@ -9,7 +9,7 @@
│ ├── @ IntegerNode (location: (1,1)-(1,2))
│ │ └── flags: decimal
│ └── @ KeywordHashNode (location: (1,4)-(1,10))
│ ├── flags: static_keys
│ ├── flags:
│ └── elements: (length: 1)
│ └── @ AssocNode (location: (1,4)-(1,10))
│ ├── key:
Expand Down
2 changes: 1 addition & 1 deletion test/prism/snapshots/seattlerb/call_arg_assoc.txt
Expand Up @@ -17,7 +17,7 @@
│ ├── @ IntegerNode (location: (1,2)-(1,3))
│ │ └── flags: decimal
│ └── @ KeywordHashNode (location: (1,5)-(1,9))
│ ├── flags: static_keys
│ ├── flags:
│ └── elements: (length: 1)
│ └── @ AssocNode (location: (1,5)-(1,9))
│ ├── key:
Expand Down
Expand Up @@ -17,7 +17,7 @@
│ ├── @ IntegerNode (location: (1,2)-(1,3))
│ │ └── flags: decimal
│ └── @ KeywordHashNode (location: (1,5)-(1,9))
│ ├── flags: static_keys
│ ├── flags:
│ └── elements: (length: 1)
│ └── @ AssocNode (location: (1,5)-(1,9))
│ ├── key:
Expand Down
2 changes: 1 addition & 1 deletion test/prism/snapshots/seattlerb/call_assoc.txt
Expand Up @@ -15,7 +15,7 @@
│ ├── flags: ∅
│ └── arguments: (length: 1)
│ └── @ KeywordHashNode (location: (1,2)-(1,6))
│ ├── flags: static_keys
│ ├── flags:
│ └── elements: (length: 1)
│ └── @ AssocNode (location: (1,2)-(1,6))
│ ├── key:
Expand Down
Expand Up @@ -15,7 +15,7 @@
│ ├── flags: ∅
│ └── arguments: (length: 1)
│ └── @ KeywordHashNode (location: (1,2)-(1,6))
│ ├── flags: static_keys
│ ├── flags:
│ └── elements: (length: 1)
│ └── @ AssocNode (location: (1,2)-(1,6))
│ ├── key:
Expand Down
Expand Up @@ -25,7 +25,7 @@
│ ├── flags: ∅
│ └── arguments: (length: 1)
│ └── @ KeywordHashNode (location: (1,4)-(1,8))
│ ├── flags: static_keys
│ ├── flags:
│ └── elements: (length: 1)
│ └── @ AssocNode (location: (1,4)-(1,8))
│ ├── key:
Expand Down
Expand Up @@ -17,7 +17,7 @@
│ ├── flags: ∅
│ └── arguments: (length: 1)
│ └── @ KeywordHashNode (location: (1,2)-(1,6))
│ ├── flags: static_keys
│ ├── flags:
│ └── elements: (length: 1)
│ └── @ AssocNode (location: (1,2)-(1,6))
│ ├── key:
Expand Down
4 changes: 2 additions & 2 deletions test/prism/snapshots/whitequark/array_assocs.txt
Expand Up @@ -7,7 +7,7 @@
│ ├── flags: ∅
│ ├── elements: (length: 1)
│ │ └── @ KeywordHashNode (location: (1,2)-(1,8))
│ │ ├── flags: static_keys
│ │ ├── flags:
│ │ └── elements: (length: 1)
│ │ └── @ AssocNode (location: (1,2)-(1,8))
│ │ ├── key:
Expand All @@ -25,7 +25,7 @@
│ ├── @ IntegerNode (location: (3,2)-(3,3))
│ │ └── flags: decimal
│ └── @ KeywordHashNode (location: (3,5)-(3,11))
│ ├── flags: static_keys
│ ├── flags:
│ └── elements: (length: 1)
│ └── @ AssocNode (location: (3,5)-(3,11))
│ ├── key:
Expand Down

0 comments on commit 01f21d5

Please sign in to comment.