Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Tuple converter regression #24609

Closed
johnnovak opened this issue Jan 11, 2025 · 3 comments · Fixed by #24611
Closed

Tuple converter regression #24609

johnnovak opened this issue Jan 11, 2025 · 3 comments · Fixed by #24611

Comments

@johnnovak
Copy link
Contributor

Description

The below code used to work on 2.0.8 and before (going back several years), but it fails with a compilation error on 2.2.0.

Not sure if this is intentional; seems like an accidental regression to me that breaks existing code.

import std/options

type
  Config* = object
    bits*: tuple[r, g, b, a: Option[int32]]

# works on 2.0.8
#
# results in error on 2.2.0
#   type mismatch: got 'int literal(8)' for '8' but expected 'Option[system.int32]'
#
converter toInt32Tuple*(t: tuple[r,g,b,a: int]): tuple[r,g,b,a: Option[int32]] =
  (some(t.r.int32), some(t.g.int32), some(t.b.int32), some(t.a.int32))

var cfg: Config
cfg.bits = (r: 8, g: 8, b: 8, a: 16)

Nim Version

2.2.0

Current Output

No response

Expected Output

No response

Known Workarounds

Don't use converters :)

Additional Information

No response
@metagn
Copy link
Collaborator

metagn commented Jan 11, 2025

!nim c

import std/options

type
  Config* = object
    bits*: tuple[r, g, b, a: Option[int32]]

# works on 2.0.8
#
# results in error on 2.2.0
#   type mismatch: got 'int literal(8)' for '8' but expected 'Option[system.int32]'
#
converter toInt32Tuple*(t: tuple[r,g,b,a: int]): tuple[r,g,b,a: Option[int32]] =
  (some(t.r.int32), some(t.g.int32), some(t.b.int32), some(t.a.int32))

var cfg: Config
cfg.bits = (r: 8, g: 8, b: 8, a: 16)

@github-actions GitHub Actions
Copy link
Contributor

🐧 Linux bisect by @metagn (collaborator)
devel 👎 FAIL

Output

Filesize 0 (0 bytes)
Duration

stable 👎 FAIL

Output

Filesize 0 (0 bytes)
Duration

2.0.10 👎 FAIL

Output

Filesize 0 (0 bytes)
Duration

2.0.0 👍 OK

Output

Filesize 90.93 Kb (93,112 bytes)
Duration

1.6.20 👍 OK

Output

Filesize 95.77 Kb (98,064 bytes)
Duration

1.4.8 👍 OK

Output

Filesize 91.95 Kb (94,152 bytes)
Duration

1.2.18 👍 OK

Output

Filesize 91.67 Kb (93,872 bytes)
Duration

1.0.10 👍 OK

Output

Filesize 86.83 Kb (88,912 bytes)
Duration

#cfd69bad1 ➡️ 🐛

Diagnostics

metagn introduced a bug at 2024-01-18 23:19:29 +0300 on commit #cfd69bad1 with message:

entiate with
`openArray[T]`), but this is too big of a refactor for now.

To compensate for this making things like `let x: (Parent, int) =
(Child(), 0)` not compile (they would crash codegen before anyway but
should still work in principle), type inference for tuple constructors
is updated such that they call `fitNode` on the fields and their
expected types, so a type conversion is generated for the individual
subtype element.

The bug is in the files:

The bug can be in the commits:

(Diagnostics sometimes off-by-one).

Stats
  • GCC 11.4.0
  • Clang 14.0.0
  • NodeJS 20.5
  • Created 2025-01-11T06:50:34Z
  • Comments 1
  • Commands nim c --run -d:nimDebug -d:nimDebugDlOpen -d:ssl -d:nimDisableCertificateValidation --forceBuild:on --colors:off --verbosity:0 --hints:off --lineTrace:off --nimcache:/home/runner/work/Nim/Nim --out:/home/runner/work/Nim/Nim/temp /home/runner/work/Nim/Nim/temp.nim

🤖 Bug found in 6 mins bisecting 1148 commits at 175 commits per second

@metagn
Copy link
Collaborator

metagn commented Jan 11, 2025

The reason is that the compiler now tries to match each individual element of the tuple constructor to the individual elements of the expected tuple type which gives an error trying to convert int to Option[int32]. The solution might be to just ignore any errors from matching the individual elements.

metagn added a commit to metagn/Nim that referenced this issue Jan 11, 2025
@metagn
ignore match errors to expected types of tuple constructor elements
6c3c423 
fixes nim-lang#24609
@metagn metagn mentioned this issue Jan 11, 2025
Merged
@Araq Araq closed this as completed in 8d0e853 Jan 15, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

ignore match errors to expected types of tuple constructor elements metagn/Nim
2 participants
@johnnovak @metagn