Support leading logical operators by kddnewton · Pull Request #3337 · ruby/prism

kddnewton · 2024-12-21T20:41:20Z

Per https://bugs.ruby-lang.org/issues/20925

Morriar · 2025-07-16T18:05:10Z

src/prism.c

+                                (peek_at(parser, following) == '.') ||
+                                (peek_at(parser, following) == '&' && peek_at(parser, following + 1) == '.')


Would it be better to use store peek_at(parser, following) into an intermediate variable?

Morriar · 2025-07-16T20:05:14Z

src/prism.c

+                                    (peek_at(parser, following) == '&' && peek_at(parser, following + 1) == '&') ||
+                                    (peek_at(parser, following) == '|' && peek_at(parser, following + 1) == '|') ||
+                                    (peek_at(parser, following) == 'a' && peek_at(parser, following + 1) == 'n' && peek_at(parser, following + 2) == 'd' && !char_is_identifier(parser, following + 3, parser->end - (following + 3))) ||
+                                    (peek_at(parser, following) == 'o' && peek_at(parser, following + 1) == 'r' && !char_is_identifier(parser, following + 2, parser->end - (following + 2)))


Same question here about all the peek_at(parser, following)

This makes it hard to do version checks against this value. The current version checks work because there are so few possible values at the moment. As an example, ruby#3337 introduces new syntax for ruby 3.5 and uses `PM_OPTIONS_VERSION_LATEST` as its version guard. Because what is considered the latest changes every year, it must later be changed to `parser->version == parser->version == PM_OPTIONS_VERSION_CRUBY_3_5 || parser->version == PM_OPTIONS_VERSION_LATEST`, with one extra version each year. With this change, the PR can instead write `parser->version >= PM_OPTIONS_VERSION_CRUBY_3_5` which is self-explanatory and works for future versions.

Earlopain · 2025-07-21T14:21:12Z

src/prism.c

-                                    (peek_at(parser, following) == '.') ||
-                                    (peek_at(parser, following) == '&' && peek_at(parser, following + 1) == '.')
-                                ))
+                                (parser->version == PM_OPTIONS_VERSION_LATEST) &&


I found this version check a bit strange since LATEST is a moving target and openend #3605 to talk about it.

PR above was merged, this should now be this

Suggested change

(parser->version == PM_OPTIONS_VERSION_LATEST) &&

(parser->version >= PM_OPTIONS_VERSION_CRUBY_3_5) &&

Earlopain · 2025-08-06T11:59:22Z

src/prism.c

-                                    (peek_at(parser, following) == '.') ||
-                                    (peek_at(parser, following) == '&' && peek_at(parser, following + 1) == '.')
-                                ))
+                                (parser->version == PM_OPTIONS_VERSION_LATEST) &&


PR above was merged, this should now be this

Suggested change

(parser->version == PM_OPTIONS_VERSION_LATEST) &&

(parser->version >= PM_OPTIONS_VERSION_CRUBY_3_5) &&

Earlopain · 2025-08-06T11:59:35Z

src/prism.c

                            LEX(PM_TOKEN_AMPERSAND_DOT);
                        }
+
+                        if (parser->version == PM_OPTIONS_VERSION_LATEST) {


Suggested change

if (parser->version == PM_OPTIONS_VERSION_LATEST) {

if (parser->version >= PM_OPTIONS_VERSION_CRUBY_3_5) {

XrXr · 2025-08-27T23:19:16Z

FYI Matz approved this feature a week ago in https://bugs.ruby-lang.org/issues/20925#note-11

ric2b · 2026-03-05T12:57:47Z

Hey @kddnewton, this change seems to turn methods called and? and or? into syntax errors when called, is that intended?

Earlopain · 2026-03-05T13:18:41Z

Hm, no. I don't think that is intentional. Do you have an account on https://bugs.ruby-lang.org to report this? If not, I can do it for you. It will need to be backported and be fixed in parse.y as well.

If you need to work around this now, you can add empty parens to your method like def logical?()

ric2b · 2026-03-05T17:13:11Z

I don't have an account there, no, feel free to report yourself.

Wrapping with parenthesis solves it, but it did look unintentional so I wanted to mention it 👍

Earlopain · 2026-03-05T19:44:51Z

I reported at https://bugs.ruby-lang.org/issues/21946, #3966 will fix this. It will probably be backported.

kddnewton force-pushed the leading-logical branch 2 times, most recently from 263df49 to 877f497 Compare December 21, 2024 20:56

kddnewton force-pushed the leading-logical branch 3 times, most recently from 75236a8 to a10344c Compare February 14, 2025 14:57

Morriar reviewed Jul 16, 2025

View reviewed changes

Earlopain mentioned this pull request Jul 21, 2025

Do not use 0 to indicate the latest ruby version to parse #3605

Merged

Earlopain reviewed Jul 21, 2025

View reviewed changes

Earlopain reviewed Aug 6, 2025

View reviewed changes

XrXr mentioned this pull request Aug 27, 2025

[3.4 backport] Prism ^D fix ruby/ruby#14358

Merged

nobu force-pushed the leading-logical branch from a10344c to 0e67140 Compare September 9, 2025 06:28

Support leading logical operators

3f58fa7

kddnewton force-pushed the leading-logical branch from 0e67140 to 3f58fa7 Compare September 12, 2025 17:41

kddnewton merged commit 8955403 into main Sep 12, 2025
60 checks passed

kddnewton deleted the leading-logical branch September 12, 2025 17:51

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support leading logical operators#3337

Support leading logical operators#3337
kddnewton merged 1 commit intomainfrom
leading-logical

kddnewton commented Dec 21, 2024

Uh oh!

Morriar Jul 16, 2025

Uh oh!

Morriar Jul 16, 2025

Uh oh!

Earlopain Jul 21, 2025

Uh oh!

Earlopain Aug 6, 2025

Uh oh!

Earlopain Aug 6, 2025

Uh oh!

Earlopain Aug 6, 2025

Uh oh!

XrXr commented Aug 27, 2025

Uh oh!

Uh oh!

ric2b commented Mar 5, 2026 •

edited

Loading

Uh oh!

Earlopain commented Mar 5, 2026

Uh oh!

ric2b commented Mar 5, 2026 •

edited

Loading

Uh oh!

Earlopain commented Mar 5, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

		(peek_at(parser, following) == '.') \|\|
		(peek_at(parser, following) == '&' && peek_at(parser, following + 1) == '.')

	(parser->version == PM_OPTIONS_VERSION_LATEST) &&
	(parser->version >= PM_OPTIONS_VERSION_CRUBY_3_5) &&

	if (parser->version == PM_OPTIONS_VERSION_LATEST) {
	if (parser->version >= PM_OPTIONS_VERSION_CRUBY_3_5) {

Conversation

kddnewton commented Dec 21, 2024

Uh oh!

Morriar Jul 16, 2025

Choose a reason for hiding this comment

Uh oh!

Morriar Jul 16, 2025

Choose a reason for hiding this comment

Uh oh!

Earlopain Jul 21, 2025

Choose a reason for hiding this comment

Uh oh!

Earlopain Aug 6, 2025

Choose a reason for hiding this comment

Uh oh!

Earlopain Aug 6, 2025

Choose a reason for hiding this comment

Uh oh!

Earlopain Aug 6, 2025

Choose a reason for hiding this comment

Uh oh!

XrXr commented Aug 27, 2025

Uh oh!

Uh oh!

ric2b commented Mar 5, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Earlopain commented Mar 5, 2026

Uh oh!

ric2b commented Mar 5, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Earlopain commented Mar 5, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

ric2b commented Mar 5, 2026 •

edited

Loading

ric2b commented Mar 5, 2026 •

edited

Loading