Skip to content

Fix radix string round trips with literal separators - #723

Open
Shubham-Padkonde wants to merge 2 commits into
intel:mainfrom
Shubham-Padkonde:fix/radix-string-literal-separators
Open

Shubham-Padkonde wants to merge 2 commits into
intel:mainfrom
Shubham-Padkonde:fix/radix-string-literal-separators

Conversation

@Shubham-Padkonde

@Shubham-Padkonde Shubham-Padkonde commented Sep 22, 2026 •

Copy link
Copy Markdown

Description & Motivation

A radix string formatted with sepChar: '', ']', or a backslash cannot be parsed with the same separator. Accept an empty separator consistently with toRadixString and escape nonempty separators before including them in the parser's regular expression. Numeric parsing and default separator behavior are unchanged.

Related Issue(s)

Fixes #722.

Testing

  • Before: the new empty, closing-bracket, and backslash cases fail; four punctuation controls pass.
  • After: dart test test/logic_value_test.dart test/logic_value_width_test.dart passes all 181 tests. Seven separator cases exercise all five supported radices, plus four-state values for non-decimal radices.
  • dart analyze lib/src/values/logic_value.dart test/logic_value_test.dart: no issues.
  • dart format run on both changed Dart files; git diff --check passes.
  • dart doc: 0 warnings, 0 errors.
  • Full dart test --fail-fast was attempted but is not green: Icarus Verilog is missing locally, and the initial run also exhausted the Windows system temporary directory. Temporary files were redirected to D: for the successful two-module run. A subsequent attempt with portable Icarus 10.1 also failed in simulation tests. Full supported-environment validation is still outstanding; the hosted workflow is waiting for maintainer approval: https://github.com/intel/rohd/actions/runs/35691054318. The code change is ready for review, but full-suite success is not claimed.

Backwards-compatibility

No public API changes. Existing separators and invalid radix-character checks remain in place; previously rejected formatter output becomes parseable.

Documentation

The parser's API comment explains literal and empty separators. A changelog entry is included.

Prepared and tested with OpenAI Codex assistance.

@Shubham-Padkonde
Shubham-Padkonde marked this pull request as ready for review September 22, 2026 05:51

@mkorbel1 mkorbel1 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This looks good to me, thank you for fixing!

@Shubham-Padkonde

Copy link
Copy Markdown
Author

The latest Run Checks retry passes markdownlint (0 errors), but the Markdown link checker cannot reach https://code.visualstudio.com/Download (status 0). Could a maintainer rerun job 106873448545? The rerun API requires repository admin rights. I have not changed code or disabled the link check for this connectivity failure. Investigated with Codex assistance.

@mkorbel1

Copy link
Copy Markdown
Contributor

Sorry about that, just restarted those runs. I think we'll remove the markdown link checks soon, they are too flakey.

This branch has not been deployed

No deployments
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.

LogicValue radix strings do not round-trip with literal or empty separators

2 participants