Skip to content

Ensure that heredoc lexing allows only valid identifiers#16548

Open
Sija wants to merge 1 commit intocrystal-lang:masterfrom
Sija:fix-15855
Open

Ensure that heredoc lexing allows only valid identifiers#16548
Sija wants to merge 1 commit intocrystal-lang:masterfrom
Sija:fix-15855

Conversation

@Sija
Copy link
Contributor

@Sija Sija commented Dec 31, 2025

Resolves #15855 (although it still allows _ as an identifier since it's valid elsewhere too)

@Blacksmoke16 Blacksmoke16 added kind:bug A bug in the code. Does not apply to documentation, specs, etc. topic:compiler:parser breaking-change May have breaking effect in some edge cases. Mostly negligible, but still noteworthy. labels Dec 31, 2025
@Sija
Copy link
Contributor Author

Sija commented Jan 12, 2026

Does this PR needs sth to be reviewed?

@ysbaddaden
Copy link
Collaborator

To be honest, I fail to see the problem, and the issue doesn't outline any trouble that may come from that.

It's also a breaking change.

Copy link
Member

@straight-shoota straight-shoota left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As mentioned in #15855 (comment), this is not critical, but still feels like the right thing to do.
I wouldn't expect violating heredoc delimiters to be used much (if at all), so the practical breaking effect is close to null.
I'd be willing to try it. If we notice anything bad happening, we can still backtrack.

@ysbaddaden ysbaddaden added this to the 1.20.0 milestone Feb 13, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

breaking-change May have breaking effect in some edge cases. Mostly negligible, but still noteworthy. kind:bug A bug in the code. Does not apply to documentation, specs, etc. topic:compiler:parser

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Heredoc identifier can accept any starting identifier value

4 participants