implementing user defined operators

[rodrigogribeiro]

No description provided.

[axic]

So this could implement 1 ether, where ether is a postfix operator (like ++) and internally would do 1 * 1 ether

[rodrigogribeiro]

So this could implement 1 ether, where ether is a postfix operator (like ++) and internally would do 1 * 1 ether

Could you give an example of using this operator?

[axic]

They are units in Solidity: https://docs.soliditylang.org/en/v0.8.36/units-and-global-variables.html#ether-units

So 1 ether == 1 * 1e18.

[rodrigogribeiro]

They are units in Solidity: https://docs.soliditylang.org/en/v0.8.36/units-and-global-variables.html#ether-units

So 1 ether == 1 * 1e18.

Thanks! It is now implemented.

[axic]

Awesome, so flexible! These units in classic are a painful exception in the parser 😅

[axic]

Since these are here -- can we remove the extra parsing stuff from Haskell?

[rodrigogribeiro]

I believe that I removed these extra parsing stuff from the parsing modules.

[axic]

Yes, just looked at it properly.

Do wonder what happens in these cases:

1. An operator is not defined, but we try a compound assignment for it.
2. An operator is redefined.
3. An operator is defined as infix and postfix, and other combinations.

Can we have tests for them?

[axic]

I'd also add a test case for redefining at the same priority, to make sure none of the parsers make a lookup separation based on that.

[rodrigogribeiro]

Added test cases for this.

[axic]

Since these special unicode code pages are allowed, there should be a test using it. Like 1 ∈ arr for finding a element 1 😅

I'm also a bit worried it may be misused, but lets see.

[rodrigogribeiro]

Well, I could remove that. But, I think it improve readability if it is not overused.

[axic]

No I meant to at least add a test for it and the have it documented later. We can always revise.

[axic]

@rodrigogribeiro opus said this, not sure if any of them are actual bugs:

I'll start by exploring the solcore repo to find the user-defined-operators code.

I couldn't build (no ghc/cabal/nix in this container), so these are from reading the diff e136159..cd44a4e (the four UDO commits) — the new OperatorScan.hs, the rewritten Expr.hs operator table, the Loader.hs operator gathering, and the Stmt.hs compound-assign desugaring.

Bugs found

1. The operator pre-scan reads comments and string literals — OperatorScan.hs:31

ops <- many (try opDeclP <|> (anySingle *> pure Nothing))

anySingle advances one character and never skips a comment as a unit (sc only runs at the very start and inside lexeme). So the scan eventually lands exactly on infixl inside a // or /* */ comment, or inside a "…" literal, and keyword "infixl" matches. The declaration becomes live.

Worst form: because duplicateOperator (OperatorScan.hs:78-84) compares symbols only — not whole declarations — merely documenting an operator breaks the file:

// The exponentiation operator:
//   infixl 70 (^^) => pow;
infixl 70 (^^) => pow;      // → SC0122: operator (^^) is declared more than once

A commented-out prefix 90 (-) => negate; in a module that imports std instead trips SC0123 via crossOperatorConflict. let s = "infixl 1 (+) => bogus"; silently rebinds +. Same defect in scanImports (:114) and scanExportModulePaths (:208).

2. Import selectors are ignored for operators — Loader.hs:217-218

selectImportedOperators :: Import -> [OperatorDecl] -> [OperatorDecl]
selectImportedOperators _ ops = ops

gatherImportedOperators text-scans each directly imported file and takes every operator it declares. So SelectOperator (parsed at Decl.hs:132, then dropped at NameResolution.hs, Loader.hs:687) does nothing: import oplib.{pow}; imports (^^) just as import oplib.{pow, (^^)}; does, hiding {…} can't exclude an operator, and import M as N; brings M's operators in unqualified. Symmetrically, ExportOperator is discarded (expandExportSpecFixed), so std.solc's export { (+), (-), … } list is decorative — and an operator can't be re-exported at all: only direct importers see it, never a module that re-exports.

3. Operator target names resolve in the use site's scope, not the declaring module's

userRow (Expr.hs:72-85) emits ExpName Nothing fun [l, r] with fun taken verbatim from the declaration. Nothing ties it to the declaring module, so importing an operator without also importing its bound function under the same name gives an unresolved-name error at the use site:

import oplib.{(^^)};   // pow not imported
… 3 ^^ 4               // → unknown name `pow`

import std.{*} hiding {and}; likewise breaks && (→ bare and). This is why the feature commit had to rewrite import std; to import std.{*}; in test/examples/comptime/ct_*.solc — a plain module import no longer makes + usable.

4. Scanner truncates qualified operator targets to two segments — OperatorScan.hs:46-53

qualFunP = do
  h  <- identifier
  mt <- optional (char '.' *> identifier)

The real parser's qualifiedName (SolcoreTypes.hs:24-27) takes unboundedly many segments. The operator table is built from the scan, so infixl 45 (<+>) => math.vec.add; scans clean (the trailing .add just falls through to optional semicolon) and binds <+> to math.vec. Silent wrong binding, no diagnostic.

5. No check that one precedence level agrees on associativity — Expr.hs:62-67

mergedOpTable groups purely by numeric precedence. makeExprParser tries pInfixR then pInfixL per level, so a level mixing InfixL and InfixR stops mid-expression and leaves the rest unconsumed. The repo already sets this up: test/operators/infixr.solc declares infixr 45 (~>) while std.solc:17-18 has infixl 45 (+) / (-). The test only writes 10 ~> 3 ~> 2, but 10 ~> 3 + 2 is an unexplainable parse error. Haskell rejects this at declaration time; here it's accepted.

6. Match-arm guard uses an empty operator table — Expr.hs:104

| sym == "|" = notFollowedBy (try (patListP [] *> symbol "=>"))

patListP's only use of the operator list is comptimePatP, so with [] a pattern like comptime 2 + 3 fails the lookahead, notFollowedBy succeeds, and | is consumed as bitwise-or instead of the arm separator — misparsing any match whose previous arm body ends in a semicolon-less expression. Should be patListP ops (thread ops through opSymP).

7. Maximal-munch guard rejects previously valid adjacent operators — Expr.hs:100-102

munchGuard = notFollowedBy (satisfy isOpChar) for symbolic operators, and isOpChar includes ! and ~. The old table used symbol "&&" with no trailing guard, so a&&!b, x||!y, a&~b all parsed before and no longer do. Nothing in-tree hits it today (I grepped std/ and test/), so it's a latent regression rather than a live break — but the guard needs to exclude symbols that can't start a longer declared operator rather than all operator characters.

8. parenOpP accepts reserved words as operator symbols — SolcoreLexer.hs:100

identSym = (:) <$> letterChar <*> many identChar

No reserved-word check (unlike identifier, :102-108). postfix 100 (if) => f; or (let) is accepted and then wrecks statement parsing wherever the keyword appears after an expression.

Smaller items

• compoundAssign / compoundAssignUnary (Stmt.hs:112-127) match on symbol only, ignoring fixity/arity — a postfix (%) makes x %= y emit a two-argument call to a unary function. They also build ExpName without locatedExpFrom, so compound assignments lose source spans that infix uses keep.
• gatherImportedOperators (Loader.hs:193-202) swallows every resolution error with catchError const (pure []), so a mistyped import silently yields no operators and then a wall of unknown-operator parse errors instead of the import diagnostic.
• scanExportModulePaths (OperatorScan.hs:200) is exported but never called — Loader.hs:155 uses the parsed exportModulePaths. Dead code.
• std.solc exports (!)…(||) but not (~) or the eight unit suffixes. Harmless today only because of bug 2; it becomes a real break the moment export filtering is implemented.
• OperatorScan.hs duplicates modPathP / externalPathP / classifyModulePath / splitQual / mkQualName / itemEntryP from Decl.hs:100-135 verbatim. Any future divergence makes the loader resolve a different module for operator gathering than for the real import.

[mbenke]

Operator imports are not working correctly, making it difficult to redefine operators:

ben@trawa:~/work/review$ cat tmp/udop.solc
import std.{addWord};

infixl 45 (+)  => addWord;

contract Ops {
  function main() -> word { 2 + 2 }
}ben@trawa:~/work/review$ esolc -f tmp/udop.solc -s -g
error[SC0101]: undefined name: Add.add
  ──> /home/ben/work/review/tmp/udop.solc:6:29
  │
6 │   function main() -> word { 2 + 2 }
  │                             ^^^^^^ unknown name
note: in: return Add.add(2, 2) ;
note: in: function main () -> word {
      return Add.add(2, 2) ;
      }
note: in: contract Ops {
      function main () -> word {
      return Add.add(2, 2) ;
      }
      }
note: module validation failed for /home/ben/work/review/tmp/udop.solc

[rodrigogribeiro]

@rodrigogribeiro opus said this, not sure if any of them are actual bugs:

I'll start by exploring the solcore repo to find the user-defined-operators code.

I couldn't build (no ghc/cabal/nix in this container), so these are from reading the diff e136159..cd44a4e (the four UDO commits) — the new OperatorScan.hs, the rewritten Expr.hs operator table, the Loader.hs operator gathering, and the Stmt.hs compound-assign desugaring.

Bugs found

1. The operator pre-scan reads comments and string literals — OperatorScan.hs:31

ops <- many (try opDeclP <|> (anySingle *> pure Nothing))

anySingle advances one character and never skips a comment as a unit (sc only runs at the very start and inside lexeme). So the scan eventually lands exactly on infixl inside a // or /* */ comment, or inside a "…" literal, and keyword "infixl" matches. The declaration becomes live.

Worst form: because duplicateOperator (OperatorScan.hs:78-84) compares symbols only — not whole declarations — merely documenting an operator breaks the file:

// The exponentiation operator:
//   infixl 70 (^^) => pow;
infixl 70 (^^) => pow;      // → SC0122: operator (^^) is declared more than once

A commented-out prefix 90 (-) => negate; in a module that imports std instead trips SC0123 via crossOperatorConflict. let s = "infixl 1 (+) => bogus"; silently rebinds +. Same defect in scanImports (:114) and scanExportModulePaths (:208).

2. Import selectors are ignored for operators — Loader.hs:217-218

selectImportedOperators :: Import -> [OperatorDecl] -> [OperatorDecl]
selectImportedOperators _ ops = ops

gatherImportedOperators text-scans each directly imported file and takes every operator it declares. So SelectOperator (parsed at Decl.hs:132, then dropped at NameResolution.hs, Loader.hs:687) does nothing: import oplib.{pow}; imports (^^) just as import oplib.{pow, (^^)}; does, hiding {…} can't exclude an operator, and import M as N; brings M's operators in unqualified. Symmetrically, ExportOperator is discarded (expandExportSpecFixed), so std.solc's export { (+), (-), … } list is decorative — and an operator can't be re-exported at all: only direct importers see it, never a module that re-exports.

3. Operator target names resolve in the use site's scope, not the declaring module's

userRow (Expr.hs:72-85) emits ExpName Nothing fun [l, r] with fun taken verbatim from the declaration. Nothing ties it to the declaring module, so importing an operator without also importing its bound function under the same name gives an unresolved-name error at the use site:

import oplib.{(^^)};   // pow not imported
… 3 ^^ 4               // → unknown name `pow`

import std.{*} hiding {and}; likewise breaks && (→ bare and). This is why the feature commit had to rewrite import std; to import std.{*}; in test/examples/comptime/ct_*.solc — a plain module import no longer makes + usable.

4. Scanner truncates qualified operator targets to two segments — OperatorScan.hs:46-53

qualFunP = do
  h  <- identifier
  mt <- optional (char '.' *> identifier)

The real parser's qualifiedName (SolcoreTypes.hs:24-27) takes unboundedly many segments. The operator table is built from the scan, so infixl 45 (<+>) => math.vec.add; scans clean (the trailing .add just falls through to optional semicolon) and binds <+> to math.vec. Silent wrong binding, no diagnostic.

5. No check that one precedence level agrees on associativity — Expr.hs:62-67

mergedOpTable groups purely by numeric precedence. makeExprParser tries pInfixR then pInfixL per level, so a level mixing InfixL and InfixR stops mid-expression and leaves the rest unconsumed. The repo already sets this up: test/operators/infixr.solc declares infixr 45 (~>) while std.solc:17-18 has infixl 45 (+) / (-). The test only writes 10 ~> 3 ~> 2, but 10 ~> 3 + 2 is an unexplainable parse error. Haskell rejects this at declaration time; here it's accepted.

6. Match-arm guard uses an empty operator table — Expr.hs:104

| sym == "|" = notFollowedBy (try (patListP [] *> symbol "=>"))

patListP's only use of the operator list is comptimePatP, so with [] a pattern like comptime 2 + 3 fails the lookahead, notFollowedBy succeeds, and | is consumed as bitwise-or instead of the arm separator — misparsing any match whose previous arm body ends in a semicolon-less expression. Should be patListP ops (thread ops through opSymP).

7. Maximal-munch guard rejects previously valid adjacent operators — Expr.hs:100-102

munchGuard = notFollowedBy (satisfy isOpChar) for symbolic operators, and isOpChar includes ! and ~. The old table used symbol "&&" with no trailing guard, so a&&!b, x||!y, a&~b all parsed before and no longer do. Nothing in-tree hits it today (I grepped std/ and test/), so it's a latent regression rather than a live break — but the guard needs to exclude symbols that can't start a longer declared operator rather than all operator characters.

8. parenOpP accepts reserved words as operator symbols — SolcoreLexer.hs:100

identSym = (:) <$> letterChar <*> many identChar

No reserved-word check (unlike identifier, :102-108). postfix 100 (if) => f; or (let) is accepted and then wrecks statement parsing wherever the keyword appears after an expression.

Smaller items

* `compoundAssign` / `compoundAssignUnary` (`Stmt.hs:112-127`) match on symbol only, ignoring fixity/arity — a `postfix` `(%)` makes `x %= y` emit a two-argument call to a unary function. They also build `ExpName` without `locatedExpFrom`, so compound assignments lose source spans that infix uses keep.

* `gatherImportedOperators` (`Loader.hs:193-202`) swallows every resolution error with `catchError const (pure [])`, so a mistyped import silently yields no operators and then a wall of unknown-operator parse errors instead of the import diagnostic.

* `scanExportModulePaths` (`OperatorScan.hs:200`) is exported but never called — `Loader.hs:155` uses the parsed `exportModulePaths`. Dead code.

* `std.solc` exports `(!)`…`(||)` but not `(~)` or the eight unit suffixes. Harmless today only because of bug 2; it becomes a real break the moment export filtering is implemented.

* `OperatorScan.hs` duplicates `modPathP` / `externalPathP` / `classifyModulePath` / `splitQual` / `mkQualName` / `itemEntryP` from `Decl.hs:100-135` verbatim. Any future divergence makes the loader resolve a different module for operator gathering than for the real import.

Thanks! I've fixed all these points (some were really tricky, due to the interaction with modules).