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.

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

Compiler crash with invalid object variant #12379

Closed
Clyybber opened this issue Oct 7, 2019 · 5 comments
Closed

Compiler crash with invalid object variant #12379

Clyybber opened this issue Oct 7, 2019 · 5 comments

Comments

@Clyybber
Copy link
Contributor

Clyybber commented Oct 7, 2019

The following snippets should provide a better error, saying that object variants/case objects are only supported with enums as discriminators.

Example

type Cobj = object
  case a: int
  of 7:
    b: bool
    bb: bool
  of 1:
    i: int
    u: uint
  else:
    uuu: uint8

var t = Cobj(a: 3)
t.i = 1

Current Output

/home/clyybber/builds/nim/compiler/nim.nim(106) nim
/home/clyybber/builds/nim/compiler/nim.nim(83) handleCmdLine
/home/clyybber/builds/nim/compiler/cmdlinehelper.nim(98) loadConfigsAndRunMainCommand
/home/clyybber/builds/nim/compiler/main.nim(188) mainCommand
/home/clyybber/builds/nim/compiler/main.nim(92) commandCompileToC
/home/clyybber/builds/nim/compiler/modules.nim(144) compileProject
/home/clyybber/builds/nim/compiler/modules.nim(85) compileModule
/home/clyybber/builds/nim/compiler/passes.nim(210) processModule
/home/clyybber/builds/nim/compiler/passes.nim(86) processTopLevelStmt
/home/clyybber/builds/nim/compiler/cgen.nim(1857) myProcess
/home/clyybber/builds/nim/compiler/ccgstmts.nim(1263) genStmts
/home/clyybber/builds/nim/compiler/ccgexprs.nim(2621) expr
/home/clyybber/builds/nim/compiler/ccgexprs.nim(2406) genStmtList
/home/clyybber/builds/nim/compiler/ccgstmts.nim(1263) genStmts
/home/clyybber/builds/nim/compiler/ccgexprs.nim(2651) expr
/home/clyybber/builds/nim/compiler/ccgstmts.nim(1247) genAsgn
/home/clyybber/builds/nim/compiler/ccgexprs.nim(2618) expr
/home/clyybber/builds/nim/compiler/ccgexprs.nim(837) genCheckedRecordField
/home/clyybber/builds/nim/compiler/ccgexprs.nim(809) genFieldCheck
/home/clyybber/builds/nim/compiler/cgen.nim(575) initLocExpr
/home/clyybber/builds/nim/compiler/ccgexprs.nim(2594) expr
/home/clyybber/builds/nim/compiler/ccgexprs.nim(126) genSetNode
/home/clyybber/builds/nim/compiler/types.nim(1394) getSize
/home/clyybber/builds/nim/compiler/sizealignoffsetimpl.nim(292) computeSizeAlign
/home/clyybber/builds/nim/compiler/int128.nim(72) toInt64
/home/clyybber/builds/nim/lib/system/assertions.nim(27) failedAssertImpl
/home/clyybber/builds/nim/lib/system/assertions.nim(20) raiseAssert
/home/clyybber/builds/nim/lib/system/fatal.nim(39) sysFatal
Error: unhandled exception: /home/clyybber/builds/nim/compiler/int128.nim(72, 11) `arg.sdata(2) == 0` out of range [AssertionError]

Changing the discriminators type to int32 traps the compiler in an infinite loop.

Additional Information

$ nim -v
Nim Compiler Version 1.0.99
@dawkot
Copy link

dawkot commented Oct 8, 2019

object variants/case objects are only supported with enums as discriminators

They are also supported with bool and ints/uints below size of 32

@Clyybber
Copy link
Contributor Author

Clyybber commented Nov 2, 2019

Doesn't seem to be fixed.

@mratsim
Copy link
Collaborator

mratsim commented Nov 2, 2019

Please also allow booleans as discriminator (didn't check lately but it used to work)

@Clyybber
Copy link
Contributor Author

Clyybber commented Nov 2, 2019

This snippet is supposed to work afaict, since the error messages in semobjconstr and the tests for discriminators seem to indicate that:
discriminators with integers with size > 16 bits should only work when the discriminator value passed to the object constructor is a const. This does not actually work though. (Replace the last line in my snippet with a valid assign)
Also when its supposed to work like that its also supposed to work for runtime discriminator values that have a type (range) that is specific enough so that the appropriate branch can be determined at compile time.

EDIT: OTOH the error here seems to stem from the fact that the backend still generates those checks and those checks rely on the size being <= 16 bits.. afaict
cc @Araq on wether size > 16 bits is supposed to work in some cases or not at all.

@Clyybber
Copy link
Contributor Author

Clyybber commented Nov 2, 2019

As this crashes:

type
 E = enum
  a = 1
  b = high(int32)
 Cobj = object
  case a: E
  of a:
    b: bool
    bb: bool
  of b:
    i: int
    u: uint

var t = Cobj(a: E.a)
t.b = true
echo t

and this:

type
 E = enum
  c = 0
  a = 1
  b = high(int32)
 Cobj = object
  case a: E
  of a:
    b: bool
    bb: bool
  of b:
    i: int
    u: uint
  else:
    uuu: uint8

var t = Cobj(a: E.a)
t.b = true
echo t

both hang up gcc(??). I think size > 16bits is just not supported.
I'll include proper errors for this in my PR. (If I am right)

@Araq Araq reopened this Nov 4, 2019
Clyybber added a commit to Clyybber/Nim that referenced this issue Nov 4, 2019
@Araq Araq closed this as completed in cf5c3f2 Nov 4, 2019
narimiran pushed a commit to narimiran/Nim that referenced this issue Nov 5, 2019
kiyolee pushed a commit to kiyolee/nim that referenced this issue Nov 7, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants