Bug #21029: Prism behavior for `defined? (;x)` differs

• Status: Closed
• Assignee: -
• Target version: -
• ruby -v: ruby 3.5.0dev (2025-01-11T03:21:57Z master 1b3037081e) +PRISM [x86_64-linux]
• Backport: 3.1: UNKNOWN, 3.2: UNKNOWN, 3.3: UNKNOWN, 3.4: UNKNOWN

[qnighy (Masaki Hara)]

Prism has a different behavior for (;expr) when used in defined? predicate:

% ./miniruby --parser=prism -e "p defined? (;x)"
nil
% ./miniruby --parser=parse.y -e "p defined? (;x)"
"expression"

Although not a significant difference, aligning either of them with the other would be better.

[kddnewton (Kevin Newton)]

• Assignee set to prism

[kddnewton (Kevin Newton)]

I think we should change parse.y to match this behavior. I will ask around.

[hsbt (Hiroshi SHIBATA)]

• Status changed from Open to Assigned

[matz (Yukihiro Matsumoto)]

I think compound expressions (expressions (including empty) concatenated by semicolons) should be “expression” as parse.y.
It makes defined? simpler. I know the current defined? behavior recursively check for defined-ness (e.g., method parameters), I don't think we need that complexity here.

Matz.

[qnighy (Masaki Hara)]

I think compound expressions (expressions (including empty) concatenated by semicolons) should be “expression” as parse.y.

Interestingly enough though:

% ruby --parser=prism -e "p defined? (x;)"
nil
% ruby --parser=parse.y -e "p defined? (x;)"
nil

% ruby --parser=prism -e "p defined? (;x)"
nil
% ruby --parser=parse.y -e "p defined? (;x)"
"expression"

[kddnewton (Kevin Newton)]

@matz (Yukihiro Matsumoto) for what it's worth, it makes it much more complicated in the Prism compiler because we don't have empty statements. So it's not a recursive situation at the moment, it's just a single statement.

As @qnighy points out, should this be "expression" for when there is a trailing ;?

[mame (Yusuke Endoh)]

Discussed at the dev meeting.

kddnewton (Kevin Newton) wrote in #note-6:

As @qnighy points out, should this be "expression" for when there is a trailing ;?

@matz (Yukihiro Matsumoto) said "yes"; defined? (x;) should also return "expression".

[kddnewton (Kevin Newton)]

I have updated Prism with https://github.com/ruby/ruby/commit/adaaa7878ebee62888bf3547d14c1db4938da88a, but the added test fails for parse.y. I haven't had a chance to look at it yet, but maybe someone else with more familiarity could.

[kddnewton (Kevin Newton)]

• Assignee deleted (prism)

[kddnewton (Kevin Newton)]

• Status changed from Assigned to Closed

Applied in changeset git|adaaa7878ebee62888bf3547d14c1db4938da88a.

---

Handle void expressions in defined?

[Bug #21029]

[S_H_ (Shun Hiraoka)]

I've fixed that defined? (x;) returns an expression when using parse.y.

https://github.com/ruby/ruby/pull/13821

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.

[kddnewton (Kevin Newton)]

@S_H_ I agree, I think the overhead is not worth it in this case and we should pretend the ; do not exist on both cases.

[S_H_ (Shun Hiraoka)]

I agree that consistent behavior between both cases would be ideal.
Given the current structure of parse.y, wrapping with NODE_BLOCK seems like the most straightforward approach to make both defined?(;x) and defined?(x;) return "expression".

With my current changes, both forms now generate the same AST nodes, which I believe improves consistency across the board.

Also, I want to clarify my earlier comment about being "skeptical" — I was specifically referring to adding new parameters to the parser_params struct, not to the NODE_BLOCK wrapping approach itself. Sorry for any confusion caused.

Right now, the defined?(x;) test is failing on parse.y, which makes it hard to assess whether changes to universal_parser.c or other components are affecting the parse.y behavior. This adds friction when trying to work on parser-related modifications.

From a maintenance standpoint, I believe fixing this failing test would help ensure that future parse.y changes can be validated more reliably.
Would you be open to keeping this change in order to maintain test consistency?