Skip to content

feat(slang): cast function call arguments to match parameter types#352

Open
hedgar2017 wants to merge 1 commit intomainfrom
feat-slang-call-argument-casting
Open

feat(slang): cast function call arguments to match parameter types#352
hedgar2017 wants to merge 1 commit intomainfrom
feat-slang-call-argument-casting

Conversation

@hedgar2017
Copy link
Copy Markdown
Contributor

@hedgar2017 hedgar2017 commented Apr 21, 2026

Store declared parameter types in function signatures and emit sol.cast for each argument whose type differs from the callee's declared parameter type. This matches the C++ reference behavior where genRValExpr receives a target type and casts accordingly.

Also cleans up inline melior:: fully-qualified paths across solx-slang by adding proper use imports for Context, BlockRef, Value, Block, Region, and IntegerType.

@hedgar2017 hedgar2017 added the ci:slang Trigger slang unit tests on PR label Apr 21, 2026
@hedgar2017 hedgar2017 force-pushed the feat-slang-call-argument-casting branch from 24b943a to ef8407c Compare April 21, 2026 04:02
@hedgar2017 hedgar2017 requested review from a team and Copilot April 21, 2026 04:03
Copy link
Copy Markdown

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 Sol dialect lowering to record full function parameter types in the MLIR context and uses those declared types at call sites to cast arguments so sol.call operand types match the callee signature (aligning more closely with solc’s “target type” rvalue emission behavior). It also simplifies several melior:: fully-qualified references by adding local use imports.

Changes:

  • Store declared parameter types (not just arity) in solx-mlir::Context function signatures and return them from resolve_function.
  • In call lowering, cast each argument to the callee’s declared parameter type before emitting sol.call.
  • Clean up melior:: path usage by importing Context, BlockRef, Value, Block, and Region where needed.

Reviewed changes

Copilot reviewed 8 out of 8 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
solx-slang/src/ast/contract/mod.rs Pre-registers parameter MLIR types (not just counts) for later call-site casting.
solx-slang/src/ast/contract/function/statement/control_flow.rs Uses Block/Region imports to reduce fully-qualified references.
solx-slang/src/ast/contract/function/mod.rs Uses imported BlockRef/Value types for clarity; no behavioral change here.
solx-slang/src/ast/contract/function/expression/operator.rs Uses imported melior::Context in operator emission signature.
solx-slang/src/ast/contract/function/expression/call/type_conversion.rs Uses imported Context/BlockRef/Value; keeps conversion logic centralized.
solx-slang/src/ast/contract/function/expression/call/mod.rs Adds argument casting prior to emitting sol.call.
solx-mlir/src/context/mod.rs Stores parameter types in signatures and returns them from resolve_function.
solx-mlir/src/context/function.rs Updates Function metadata to store parameter_types: Vec<Type>.

Comment thread solx-slang/src/ast/contract/mod.rs Outdated
Comment thread solx-slang/src/ast/contract/function/expression/call/mod.rs
@hedgar2017 hedgar2017 self-assigned this Apr 21, 2026
@hedgar2017 hedgar2017 force-pushed the feat-slang-call-argument-casting branch 2 times, most recently from 304913e to ab59f37 Compare April 21, 2026 10:15
Base automatically changed from replace-type-cache-with-fields to main April 21, 2026 18:53
@hedgar2017 hedgar2017 force-pushed the feat-slang-call-argument-casting branch 2 times, most recently from 9d62e0f to 4ab5e3a Compare April 28, 2026 06:04
@abinavpp
Copy link
Copy Markdown
Contributor

It looks like resolve_function maintains a name-to-MLIR-def map to resolve callees during call codegen. This seems fragile - matching by name + argument types reimplements overload resolution that the frontend should have already done. Does Slang provide a way to resolve the callee directly from the call expression (something like a resolve_to_definition() on the callee operand)?

If it does, we should map the resolved AST definition to its MLIR op, keyed by something unique to the definition node (like its node ID), rather than doing string-based lookup.

@hedgar2017
Copy link
Copy Markdown
Contributor Author

@abinavpp that makes total sense, and I want to do this migration. But this PR's job is only emitting the casts.
The lookup change is orthogonal to that and doesn't remove the cast work.

Happy to fold it in here or do it as a follow-up. What do you prefer?

Register declared parameter types in function signatures and resolve
overloads by exact type match instead of arity. Cast arguments using
TypeConversion to correctly handle bool (cmp != 0) and address
(truncate to ui160 + address_cast). Extract resolve_function_types
to deduplicate Slang-to-MLIR type resolution.
@hedgar2017 hedgar2017 force-pushed the feat-slang-call-argument-casting branch from 4ab5e3a to 03a1782 Compare April 28, 2026 16:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ci:slang Trigger slang unit tests on PR

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants