Skip to content

Conversation

@amkCha
Copy link
Collaborator

@amkCha amkCha commented Nov 25, 2025

…r ternary operator using a const

Addresses issue #1284 with some limitations :

  • tests cannot run on Koala bear field
  • the register against which it checks cannot be bigger than u128

Use this feature in TRM module - go-corset side


Note

Adds label-capable ternary (IfThenElse) parsing and linker resolution (also for IfGoto), and wires an example test plus TRM module usage.

  • Assembler/Parser:
    • Support ternary assignments by parsing RHS via parseAtomicExpr; capture constant labels and reject register RHS.
    • Emit macro.IfThenElse with Label for RHS constants.
  • Assembler/Linker:
    • Resolve constant labels for macro.IfGoto and macro.IfThenElse via getLabelValue, setting Constant/Right.
  • Macro:
    • Extend macro.IfThenElse to include Label field.
  • Tests:
    • Add testdata/asm/util/ternary.zkasm and .accepts with a simple const-based ternary; new Test_AsmUtil_Ternary (excludes KOALABEAR_16).
  • Bench/TRM:
    • Define P256_VERIFY_ADDRESS and use it in ternary instead of literal 0x100.

Written by Cursor Bugbot for commit bec466c. This will update automatically on new commits. Configure here.

Copy link
Collaborator

@DavePearce DavePearce left a comment

Choose a reason for hiding this comment

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

Overall, the change looks good to me. I think we ended up in the right place for what you wanted. You just need to address the syntax error issue, and fix the linting.

rhs = e.Constant
label = e.Label
case *expr.RegAccess:
panic(fmt.Sprintf("ternary operator does not support register on the rhs"))
Copy link
Collaborator

Choose a reason for hiding this comment

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

No, don't do a panic. You need to return a proper SyntaxError so that we get a useful error message which highlights where the problem is.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Noted 👌 let me know if it's ok to go backwards to point to the right token or if it's better to print the error on the next token with p.lookahead()

@amkCha amkCha force-pushed the 1284-support-labels-in-ternary-operator branch from 526fd2c to 968ae5c Compare November 26, 2025 10:53
@amkCha amkCha marked this pull request as ready for review November 26, 2025 10:53
Copy link
Collaborator

@DavePearce DavePearce left a comment

Choose a reason for hiding this comment

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

LGTM

@DavePearce DavePearce merged commit 1e72a8b into main Nov 26, 2025
31 of 36 checks passed
@DavePearce DavePearce deleted the 1284-support-labels-in-ternary-operator branch November 26, 2025 20:06
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