[mlir][Sol] Replace int64_t sentinel for dynamic Sol_ArrayType size with std::optional<APInt>

[PavelKopyl]

NomicFoundation/solx-solidity#142

Replace the int64_t dimension with a -1 sentinel for dynamic arrays:
int64_t can neither hold storage-array dimensions up to 2^256-1 nor
reliably distinguish them from the sentinel. Store the size as
std::optional (nullopt = dynamic) via a custom Sol_OptAPIntParam,
needed because ODS's OptionalParameter asserts on mismatched
bit widths in the generated comparator.

[Copilot]

Pull request overview

This PR updates the Sol dialect’s sol.array type to represent static array dimensions as std::optional<llvm::APInt> (nullopt = dynamic, value = 256-bit static size), removing the previous int64_t “-1 sentinel” approach that could not correctly represent Solidity’s allowed uint[(2**256)-1] bounds. It also introduces a custom ODS parameter (Sol_OptAPIntParam) with a safe comparator to avoid APInt::operator== bitwidth assertions when used under std::optional.

Changes:

• Replace Sol_ArrayType size parameter from int64_t (with -1 sentinel) to std::optional<llvm::APInt> and add helper APIs (isDynSized(), getSize(), getSmallSize()).
• Update parsing/printing and multiple lowering/utilities to use the new APIs (including APInt comparisons where needed).
• Add an ODS parameter wrapper with a comparator that guards bitwidth equality before comparing APInt values.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 4 comments.

Show a summary per file

File	Description
mlir/lib/Dialect/Sol/SolBase.cpp	Updates sol.array parse/print and storage slot sizing to use optional APInt sizes.
mlir/lib/Conversion/SolToYul/TypeConverter.cpp	Switches stack array lowering to use getSmallSize() for LLVM array element counts.
mlir/lib/Conversion/SolToYul/SolToYul.cpp	Updates array bounds checks and memory allocation sizing to match new array-size representation.
mlir/lib/Conversion/SolToYul/EVMUtil.cpp	Replaces size computations with getSmallSize()/APInt comparisons for ABI/memory helpers and array copy logic.
mlir/include/mlir/Dialect/Sol/SolBase.td	Introduces Sol_OptAPIntParam and updates Sol_ArrayType parameter + helper methods.

Comments suppressed due to low confidence (1)

mlir/lib/Conversion/SolToYul/EVMUtil.cpp:3360

• arrTy.getSmallSize() * 32 is evaluated as a signed multiplication and can overflow (undefined behavior) before being wrapped into an i256 constant. Use checked multiplication (or APInt arithmetic) before converting to an i256 constant.

      dstAddr = genMemAlloc(bExt.genI256Const(arrTy.getSmallSize() * 32), loc);
      ret = dstAddr;
      srcAddr = addr;
      size = bExt.genI256Const(arrTy.getSize());
      guards.requireFixedArraySpan(srcAddr, arrTy);

---

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

[abinavpp]

getSmallSize + getSize is confusing. how about just getSize? and those non storage contexts could use getZExtValue (which, iirc, asserts?)? optionally, we could add a type verifier that only allows >= u64.max values for storage data-locations.

thanks for working on this!

[PavelKopyl]

getSmallSize + getSize is confusing. how about just getSize? and those non storage contexts could use getZExtValue (which, iirc, asserts?)? optionally, we could add a type verifier that only allows >= u64.max values for storage data-locations.

thanks for working on this!

Thanks, will change that.

[github-actions]

✅ With the latest revision this PR passed the C/C++ code formatter.

[Copilot]

Pull request overview

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

[Copilot]

Pull request overview

Copilot reviewed 9 out of 9 changed files in this pull request and generated no new comments.

Suppressed comments (4)

mlir/lib/Conversion/SolToYul/EVMUtil.cpp:3767

• This allocation size is computed with host uint64_t arithmetic (getZExtValue() * 32) before being wrapped into an i256 constant. For very large fixed array sizes, the host multiplication can overflow and wrap, causing an incorrect (too small) allocation size to be emitted. Use APInt arithmetic for the * 32 computation to preserve the full value and let the existing runtime guards trigger appropriately.

      dstAddr = genMemAlloc(
          bExt.genI256Const(arrTy.getSize().getZExtValue() * 32), loc);
      ret = dstAddr;

mlir/lib/Conversion/SolToYul/SolToYul.cpp:2297

• genMemAlloc is being called with an AllocSize computed via host uint64_t arithmetic (getZExtValue() * 32). With the new APInt sizes, fixed arrays can be large enough that this overflows/wraps (and also narrows to int64_t), producing an incorrect allocation size. Prefer the genMemAlloc(Value) overload and compute sizeInBytes in i256 IR, which will interact correctly with the existing runtime overflow/too-large guards.

        size = bExt.genI256Const(arrTy.getSize());
        dstAddr = evmB.genMemAlloc(arrTy.getSize().getZExtValue() * 32);
        dstDataAddr = dstAddr;

mlir/lib/Conversion/SolToYul/EVMUtil.cpp:862

• getMallocSize converts the 256-bit array size to uint64_t and then to int64_t implicitly via static_cast<int64_t>(...). With the new APInt-backed sizes, non-storage arrays can legally be larger than INT64_MAX (up to 2^64-1), which would make this cast implementation-defined and the returned allocation size incorrect (potentially negative). Consider doing the multiplication in APInt and asserting that the byte size fits AllocSize before converting.

  if (auto arrayTy = dyn_cast<sol::ArrayType>(ty)) {
    assert(!arrayTy.isDynSized());
    return static_cast<int64_t>(arrayTy.getSize().getZExtValue()) * 32;
  }

mlir/lib/Conversion/SolToYul/EVMUtil.cpp:787

• This bounds check computes the fixed-array span using host uint64_t arithmetic (getZExtValue() * ...). For sizes > 2^64/stride, this multiplication will wrap in C++ before creating the i256 constant, potentially making the end-address check unsound for very large (but verifier-legal) non-storage arrays. Compute the span in APInt (256-bit) to avoid host overflow.

This issue also appears on line 3765 of the same file.

    Value endAddr = b.create<yul::AddOp>(
        loc, baseAddr,
        bExt.genI256Const(arrTy.getSize().getZExtValue() *
                          evm::getCallDataHeadSize(arrTy.getEltType())));

[abinavpp]

lgtm, thank you!