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 crashes when using string as object variant selector with else branch #14189

Closed
metagn opened this issue May 1, 2020 · 2 comments
Closed

Comments

@metagn
Copy link
Collaborator

metagn commented May 1, 2020

Example

type Obj = object
  case str: string
  of "abc": discard
  else: discard

Current Output

Error: internal error: invalid kind for lastOrd(tyString)
Traceback (most recent call last)
compiler/nim.nim(104) nim
compiler/nim.nim(81) handleCmdLine
compiler/cmdlinehelper.nim(91) loadConfigsAndRunMainCommand
compiler/main.nim(196) mainCommand
compiler/main.nim(98) commandCompileToC
compiler/modules.nim(143) compileProject
compiler/modules.nim(84) compileModule
compiler/passes.nim(210) processModule
compiler/passes.nim(86) processTopLevelStmt
compiler/sem.nim(599) myProcess
compiler/sem.nim(567) semStmtAndGenerateGenerics
compiler/semstmts.nim(2308) semStmt
compiler/semexprs.nim(1014) semExprNoType
compiler/semexprs.nim(2776) semExpr
compiler/semstmts.nim(2248) semStmtList
compiler/semexprs.nim(2781) semExpr
compiler/semstmts.nim(1423) semTypeSection
compiler/semstmts.nim(1265) typeSectionRightSidePass
compiler/semtypes.nim(1916) semTypeNode
compiler/semtypes.nim(882) semObjectNode
compiler/semtypes.nim(755) semRecordNodeAux
compiler/semtypes.nim(748) semRecordNodeAux
compiler/semtypes.nim(685) semRecordCase
compiler/types.nim(841) lengthOrd
compiler/types.nim(802) lastOrd
compiler/msgs.nim(564) internalError
compiler/msgs.nim(442) rawMessage
compiler/msgs.nim(439) rawMessage
compiler/msgs.nim(356) handleError
compiler/msgs.nim(346) quit

Expected Output

Compiles or disallowed. There is an error message that says Error: selector must be of an ordinal type, float or string if you make the selector type anything but those types.

Possible Solution

To disallow, change:

Nim/compiler/semtypes.nim

Lines 673 to 681 in 9c33bca

of tyFloat..tyFloat128, tyString, tyError:
discard
of tyRange:
if skipTypes(typ[0], abstractInst).kind in shouldChckCovered:
chckCovered = true
of tyForward:
errorUndeclaredIdentifier(c, n[0].info, typ.sym.name.s)
elif not isOrdinalType(typ):
localError(c.config, n[0].info, "selector must be of an ordinal type, float or string")

to:

  of tyFloat..tyFloat128, tyError:
    discard
  ...
  elif not isOrdinalType(typ):
    localError(c.config, n[0].info, "selector must be an ordinal type or float")
$ nim -v
Nim Compiler Version 1.3.1
@metagn metagn changed the title Compiler crashes when using string as object variant selector Compiler crashes when using string as object variant selector with else branch May 1, 2020
@disruptek
Copy link
Contributor

I believe the consensus is that strings should be disallowed in object variant typedefs. A pull to that effect would let us measure breakage, to some extent. It might need a documentation update, too.

@metagn
Copy link
Collaborator Author

metagn commented May 6, 2020

Strings in object variants are not documented, so no need to update anything there. From the manual:

This analysis only works for immutable discriminators of an ordinal type

This error message is just randomly misleading. It's really easy to remove it, someone can make a PR with the proposed change above if they want

@ghost ghost added the Compiler Crash label Jul 25, 2020
bung87 added a commit to bung87/Nim that referenced this issue Jul 26, 2020
@ghost ghost added the Object Variants label Jul 26, 2020
@Araq Araq closed this as completed in 191c388 Jul 27, 2020
narimiran pushed a commit that referenced this issue Jul 29, 2020
(cherry picked from commit 191c388)
narimiran pushed a commit that referenced this issue Jul 29, 2020
(cherry picked from commit 191c388)
mildred pushed a commit to mildred/Nim that referenced this issue Jan 11, 2021
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

2 participants