8000 Add state_dependent_path_declaration so that `ifnone` can be parsed by FlinkbaumFAU · Pull Request #5121 · YosysHQ/yosys · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Add state_dependent_path_declaration so that ifnone can be parsed #5121

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

FlinkbaumFAU
Copy link

The parser crashed when an ifnone-statement occured. See Bug #5092 .

The state_dependen_path_declaration from the specification was added to the parser file.

@FlinkbaumFAU FlinkbaumFAU requested a review from zachjs as a code owner May 14, 2025 19:19
@@ -6,7 +6,7 @@ GENFILES += frontends/verilog/verilog_lexer.cc

frontends/verilog/verilog_parser.tab.cc: frontends/verilog/verilog_parser.y
$(Q) mkdir -p $(dir $@)
$(P) $(BISON) -Wall -Werror -o $@ -d -r all -b frontends/verilog/verilog_parser $<
$(P) $(BISON) -Wall -Wcex -Werror -o $@ -d -r all -b frontends/verilog/verilog_parser $<
Copy link
Member

Choose a reason for hiding this comment

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

From https://www.gnu.org/software/bison/manual/html_node/Diagnostics.html:

cex
Provide counterexamples for conflicts. See Generation of Counterexamples. Counterexamples take time to compute. The option -Wcex should be used by the developer when working on the grammar; it hardly makes sense to use it in a CI.

I don't think this should be included.

module_path_expression:
module_path_primary
// Flatten out unary_operator to avoid shift/reduce conflict
| '!' attr module_path_primary { delete $2; }
Copy link
Member

Choose a reason for hiding this comment

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

Based on the existing code, these need to be freeattr($2); instead to avoid a memory leak (not that I'm sure why they're being deleted).

@KrystalDelusion
Copy link
Member

Also could you please add test(s) for the ifnone parsing? There are some existing tests in tests/various/specify.{v,ys} that you can expand/reference.

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
0