dialect: (riscv) Throw parser error if immediate is not given

[kingiler]

No description provided.

[codecov]

Codecov Report

Patch coverage: 100.00% and no project coverage change.

Comparison is base (d6676e2) 89.73% compared to head (c800fd9) 89.74%.

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #1371   +/-   ##
=======================================
  Coverage   89.73%   89.74%           
=======================================
  Files         200      200           
  Lines       25700    25713   +13     
  Branches     3852     3854    +2     
=======================================
+ Hits        23063    23077   +14     
  Misses       2025     2025           
+ Partials      612      611    -1     

Files Changed	Coverage Δ
tests/dialects/test_riscv.py	100.00% <100.00%> (ø)
xdsl/dialects/riscv.py	85.00% <100.00%> (+0.16%)	⬆️

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[superlopuh]

Can you please make it non-optional?

[kingiler]

I'm not sure if that is possible. Immediate is either integer or string literal.

[superlopuh]

I would recommend adding a parse_immediate and parse_optional_immediate helper pair that return either an int or a string literal (or None)

[superlopuh]

I would recommend something like this:

Suggested change

	def _parse_int_or_str_literal(parser: Parser, msg: str) -> int | str:
	if (term := parser.parse_optional_integer()) is not None:
	return term
	if (term := parser.parse_optional_str_literal()) is not None:
	return term
	parser.raise_error(msg)
	def _parse_optional_immediate_value(parser: Parser, integer_type: type[IntegerType]) -> IntegerAttr | LabelAttr | None:
	"""
	Parse an optional immediate value. If an int is parsed, an integer attr with the specified type is created.
	"""
	if (term := parser.parse_optional_integer()) is not None:
	return IntegerAttr(immediate, type)
	if (term := parser.parse_optional_str_literal()) is not None:
	return LabelAttr(term)
	return None
	def _parse_immediate_value(parser: Parser, integer_type: type[IntegerType]) -> IntegerAttr | LabelAttr:
	return parser.expect(lambda: _parse_optional_literal_value(parser, integer_type), "literal value")

It feels like the error message should be the same for all clients, and not a part of the helper API. We might have optional immediate values so an optional helper would also be nice.

[superlopuh]

LGTM, now just need some tests :)