Skip to content

Accept multibase string literals in CSR enum value parsing#231

Merged
james-ball-qualcomm merged 5 commits intoadd_behavior_to_warl_enumfrom
copilot/sub-pr-230
Mar 24, 2026
Merged

Accept multibase string literals in CSR enum value parsing#231
james-ball-qualcomm merged 5 commits intoadd_behavior_to_warl_enumfrom
copilot/sub-pr-230

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented Mar 24, 2026

CSR enum parsing fatalied on quoted hex/binary literals (e.g. "0x1", "0b10") in enum.legal and enum.illegal-write-return, despite the schema permitting multiBaseValue inputs. Behavior depended on whether the author quoted the value in YAML.

Changes

  • enum.legal parsing: Broadened type check from int-only to (int, str), delegating to the existing parse_multibase_int helper for string resolution.
  • enum.illegal-write-return parsing: Same fix applied — accepts quoted hex/binary strings via parse_multibase_int.
  • Error messages: Updated to reflect the expanded accepted types (integer, hex string, or binary string).

Example — now valid YAML

enum:
  legal: ["0x0", "0x1", "0b10"]   # previously fatal; now parsed correctly
  illegal-write-return: "0xFF"    # previously fatal; now parsed correctly

📍 Connect Copilot coding agent with Jira, Azure Boards or Linear to delegate work to Copilot in one click without leaving your project management tool.

Copilot AI and others added 4 commits March 24, 2026 20:06
…rn parsing

Co-authored-by: james-ball-qualcomm <140646808+james-ball-qualcomm@users.noreply.github.com>
Agent-Logs-Url: https://github.com/riscv/docs-resources/sessions/11ad9d00-d632-4b46-9c65-ee154ee0338e
Signed-off-by: James Ball <jameball@qti.qualcomm.com>
Co-authored-by: james-ball-qualcomm <140646808+james-ball-qualcomm@users.noreply.github.com>
Agent-Logs-Url: https://github.com/riscv/docs-resources/sessions/11ad9d00-d632-4b46-9c65-ee154ee0338e
Copilot AI changed the title [WIP] [WIP] Address feedback on changing 'type' to 'enum' object in PR #230 Accept multibase string literals in CSR enum value parsing Mar 24, 2026
@james-ball-qualcomm james-ball-qualcomm marked this pull request as ready for review March 24, 2026 23:55
Copilot AI review requested due to automatic review settings March 24, 2026 23:55
@james-ball-qualcomm james-ball-qualcomm merged commit 3ca5f6b into add_behavior_to_warl_enum Mar 24, 2026
2 checks passed
@james-ball-qualcomm james-ball-qualcomm deleted the copilot/sub-pr-230 branch March 24, 2026 23:55
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR updates CSR enum parsing in tools/create_params.py to correctly accept schema-permitted multibase string literals (e.g., "0x1", "0b10") in enum.legal, avoiding failures that depended on YAML quoting behavior.

Changes:

  • Allow enum.legal entries to be either int or str, converting via the existing parse_multibase_int helper.
  • Normalize enum.legal values to integers during parsing (instead of passing raw values through).
  • Adjust validation paths to support multibase inputs (with one remaining error-message inconsistency noted below).

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +654 to 658
if not isinstance(value, (int, str)) or isinstance(value, bool):
fatal(
f"CSR {representative_name} in {def_filename} has non-integer "
f"value in enum.legal: {value!r}"
f"CSR {representative_name} in {def_filename} has invalid value in enum.legal: "
f"{value!r}; expected integer, hex string, or binary string"
)
Copy link

Copilot AI Mar 24, 2026

Choose a reason for hiding this comment

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

The enum.legal type-check now allows strings, but the fatal message still says “non-integer value”. This is misleading (and also inaccurate for other invalid types like dict/list). Please update the message to reflect the accepted inputs (int or multibase string) or rely on parse_multibase_int for the error so the wording stays consistent.

Copilot uses AI. Check for mistakes.
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.

3 participants