Skip to content

implements EnumToStrEntry for nim ic#25614

Merged
Araq merged 3 commits intodevelfrom
pr_ic_re
Mar 19, 2026
Merged

implements EnumToStrEntry for nim ic#25614
Araq merged 3 commits intodevelfrom
pr_ic_re

Conversation

@ringabout
Copy link
Member

@ringabout ringabout commented Mar 17, 2026

type
  NodeKind = enum
    nkInt, nkStr, nkAdd

  Node = object
    case kind: NodeKind
    of nkInt: intVal: int
    of nkStr: strVal: string
    of nkAdd: left, right: ref Node

proc newInt(v: int): ref Node =
  new(result)
  result[] = Node(kind: nkInt, intVal: v)

let n = newInt(42)
echo n.intVal

gives unhandled exception: key not found: (module: 68, item: 3) [KeyError]

@ringabout ringabout changed the title implements EnumToStrEntry implements EnumToStrEntry for nim ic Mar 17, 2026
@ringabout ringabout marked this pull request as ready for review March 17, 2026 10:46
Copilot AI review requested due to automatic review settings March 17, 2026 10:46
Copy link
Contributor

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 fixes a crash in the NIF-based incremental compilation workflow (used by nim ic via the nim m / nim nifc steps) by properly persisting and reloading enum-to-string procs through the NIF index, and adds a regression test covering object variants.

Changes:

  • Serialize EnumToStrEntry log entries into NIF (ast2nif.nim) so enum-to-string procs are recorded in module NIFs.
  • Load EnumToStrEntry entries from the NIF index into the module graph and make getToStringProc fall back to key-based lookup (modulegraphs.nim).
  • Add an ic regression test for enum discriminants in case objects (tests/ic/tenum.nim).

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

File Description
tests/ic/tenum.nim Regression test reproducing the enum/case-object scenario that previously crashed under nim ic.
compiler/modulegraphs.nim Adds a key-based cache for enum-to-string procs loaded from NIF and uses it as a fallback in getToStringProc.
compiler/ast2nif.nim Implements writing EnumToStrEntry into the NIF output so it can be restored later.

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

if result == nil and g.config.cmd in {cmdNifC, cmdM}:
let key = typeKey(t, g.config, loadTypeCallback, loadSymCallback)
result = g.loadedEnumToStringProcs.getOrDefault(key)
assert result != nil
@ringabout
Copy link
Member Author

There is an error error: passing argument 1 of ‘eqdestroy___OOZOOZsysma2dyk_u70’ from incompatible pointer type [-Wincompatible-pointer-types], I will solve it later

@ringabout
Copy link
Member Author

ringabout commented Mar 19, 2026

There is an error error: passing argument 1 of ‘eqdestroy___OOZOOZsysma2dyk_u70’ from incompatible pointer type [-Wincompatible-pointer-types]

I find the cause of it => #25619
the element type of seqs is skipped by typekeys

@Araq Araq merged commit a7e0065 into devel Mar 19, 2026
18 checks passed
@Araq Araq deleted the pr_ic_re branch March 19, 2026 06:35
@github-actions
Copy link
Contributor

Thanks for your hard work on this PR!
The lines below are statistics of the Nim compiler built from a7e0065

Hint: mm: orc; opt: speed; options: -d:release
190128 lines; 11.770s; 802.941MiB peakmem

Araq pushed a commit that referenced this pull request Mar 20, 2026
…types for `nim ic` (#25621)

fixes #25620

This pull request includes a fix to the type key generation logic in the
compiler and updates to a test file to cover additional language
features. The most important changes are summarized below:

### Compiler logic fix

* In `compiler/typekeys.nim`, the `typeKey` procedure was updated to
iterate over all elements in `t.sonsImpl` starting from index 0 instead
of 1, ensuring that all type sons are considered during type key
generation.

### Test suite improvements

* The test file `tests/ic/tenum.nim` was renamed to
`tests/ic/tmiscs.nim`, and its output expectations were updated to
reflect the new test cases.
* Added new test cases to `tests/ic/tmiscs.nim` to cover sink and move
semantics, including the definition of a `BigObj` type and a `consume`
procedure that demonstrates moving and consuming large objects.

```nim
# Sink and move semantics
type
  BigObj = object
    data: seq[int]

proc consume(x: sink BigObj) =
  echo x.data.len

var b = BigObj(data: @[1, 2, 3, 4, 5])
consume(move b)
```

gives

```
error: passing 'tySequence__qwqHTkRvwhrRyENtudHQ7g' (aka 'struct tySequence__qwqHTkRvwhrRyENtudHQ7g') to parameter of incompatible type 'tySequence__cTyVHeHOWk5jStsToosJ8Q' (aka 'struct tySequence__cTyVHeHOWk5jStsToosJ8Q')
   84 |         eqdestroy___sysma2dyk_u75((*dest_p0).data);
```

follows up #25614
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