Skip to content

verilog: fix string literal regular expression #5187

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Jun 19, 2025
Merged

Conversation

garytwong
Copy link
Contributor

@garytwong garytwong commented Jun 19, 2025

What are the reasons/motivation for this change?
Fix the bug reported in #5160, where a closing quote on a string literal failed to terminate the string when preceded by an escaped backslash.
Originally posted by @mmicko in #5160 (comment)

Explain how this is achieved.
Properly escape the backslash in the regular expression in verilog_lexer.l.

If applicable, please suggest to reviewers how they can test the change.
See the included regression test tests/verilog/bug5160.v, or @mmicko's test case in the original report.

With this fix applied, both cases work properly. Before the fix, parsing would fail with a spurious syntax error.

A backslash was improperly quoted, causing string literal matching
to fail when the final token before a closing quote was an escaped
backslash.
Test for bug triggered by escaped backslash immediately before
closing quote (introduced in ca7d94a and fixed by 40aa7ea).
@garytwong garytwong requested a review from zachjs as a code owner June 19, 2025 13:57
@zachjs zachjs merged commit 834a729 into YosysHQ:main Jun 19, 2025
24 of 25 checks passed
@garytwong
Copy link
Contributor Author

While fixing this, I also noticed another bug related to escape sequences in string literals: yosys doesn't seem to process escaped newlines (as in string literal split over multiple lines, not the "\n" sequence) according to IEEE 1800-2012 sec 5.9. (It looks like such strings are accepted, but the resulting value can be wrong.)

However, since that bug predates the recent changes, I'll plan to submit a new PR once I have time to investigate.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants