Make defined? (x;) return expression when using parse.y parser
#13821
Conversation
| @@ -13013,6 +13021,9 @@ kwd_append(rb_node_kw_arg_t *kwlist, rb_node_kw_arg_t *kw) | |||
| static NODE * | |||
| new_defined(struct parser_params *p, NODE *expr, const YYLTYPE *loc) | |||
| { | |||
| int had_trailing_semicolon = p->ctxt.has_trailing_semicolon; | |||
| p->ctxt.has_trailing_semicolon = 0; | |||
There was a problem hiding this comment.
I suspect that the has_trailing_semicolon saved at begin_defined should be restored after new_defined() calls.
There was a problem hiding this comment.
Are you suggesting that I should either remove this line where I assign 0 to has_trailing_semicolon, or restore the original value by assigning had_trailing_semicolon back to has_trailing_semicolon?
If so, I'll remove this initialization as it was added by mistake.
There was a problem hiding this comment.
I suggest that has_trailing_semicolon should be restored from the context saved at begin_defined, after new_defined() is called.
diff --git a/parse.y b/parse.y
index 3138061b988..a92df0ae81f 100644
--- a/parse.y
+++ b/parse.y
@@ -4041,6 +4041,7 @@ arg : asgn(arg_rhs)
{
p->ctxt.in_defined = $3.in_defined;
$$ = new_defined(p, $4, &@$);
+ p->ctxt.has_trailing_semicolon = $3.has_trailing_semicolon;
/*% ripper: defined!($:4) %*/
}
| def_endless_method(endless_arg)
@@ -4428,6 +4429,7 @@ primary : inline_primary
{
p->ctxt.in_defined = $4.in_defined;
$$ = new_defined(p, $5, &@$);
+ p->ctxt.has_trailing_semicolon = $4.has_trailing_semicolon;
/*% ripper: defined!($:5) %*/
}
| keyword_not '(' expr rparenOr, pass the struct lex_context to new_defined and restore it and in_defined together there?
There was a problem hiding this comment.
Thank you for the reply and I understand.
I've restore has_trailing_semicolon after new_defined().
Or, pass the struct lex_context to new_defined and restore it and in_defined together there?
I think it would be better not to do that.
Instead, I thought it's better to restore has_trailing_semicolon after calling new_defined().
If we pass the struct lex_context to new_defined(), we might need to create a temporary copy or something similar.
It would be simpler and better to just reassign the value in the action part.
S-H-GAMELINKS
force-pushed
the
follow-up/defined-returns-expression-in-parsey
branch
3 times, most recently
from
927797b to
611879c
Compare
Follow up [Bug #21029]. Currently, `defined? (x;)` returns `expression` when using Prism parser. See: - ruby#12949 - https://bugs.ruby-lang.org/issues/21029 However, `defined? (x;)` returns nil when using parse.y, as reported in bug ticket comment and test-all (when using parse.y parser) test result. This change adds a context flag to track trailing semicolons in defined? scope. When a trailing semicolon is detected within a defined? scope, the generated AST node is wrapped with NODE_BLOCK. This change ensures consistent behavior with `defined? (;x)` .
S-H-GAMELINKS
force-pushed
the
follow-up/defined-returns-expression-in-parsey
branch
from
611879c to
7d1969d
Compare
Follow up [Bug #21029].
Currently,
defined? (x;)returnsexpressionwhen using Prism parser.See:
However,
defined? (x;)returns nil when using parse.y, as reported in bug ticket comment and test-all (when using parse.y parser) test result.This change adds a context flag to track trailing semicolons in defined? scope. When a trailing semicolon is detected within a defined? scope, the generated AST node is wrapped with NODE_BLOCK. This change ensures consistent behavior with
defined? (;x).However, I have a little skeptical about whether this change is a good idea. If any other good suggestions, I would appreciate it if you could comment.