implement RFCs/294 ; disallow enum <=> enum conversion (#16351)

* fix https://github.com/nim-lang/RFCs/issues/294 ; disallow enum <=> enum conversion
* fix the runnableExamples that was the instigator of this RFC
* legacy -d:nimLegacyConvEnumEnum
* use -d:nimLegacyConvEnumEnum in important_package nimgame2
* add test for enum cast
* improve changelog
* add changelog: Changes affecting backward compatibility
* cleanup changelog
* fix changelog

changelog.md

@@ -2,6 +2,40 @@
 
 
 
+## Changes affecting backward compatibility
+
+- `repr` now doesn't insert trailing newline; previous behavior was very inconsistent,
+  see #16034. Use `-d:nimLegacyReprWithNewline` for previous behavior.
+
+- An enum now can't be converted to another enum directly, you must use `ord` (or `cast`, but
+  compiler won't help if you misuse it).
+  ```
+  type A = enum a1, a2
+  type B = enum b1, b2
+  doAssert not compiles(a1.B)
+  doAssert compiles(a1.ord.B)
+  ```
+  for a transition period, use `-d:nimLegacyConvEnumEnum`.
+
+- Type mismatch errors now show more context, use `-d:nimLegacyTypeMismatch` for previous
+  behavior.
+
+- `echo` and `debugEcho` will now raise `IOError` if writing to stdout fails.  Previous behavior
+  silently ignored errors.  See #16366.  Use `-d:nimLegacyEchoNoRaise` for previous behavior.
+
+- `math.round` now is rounded "away from zero" in JS backend which is consistent
+  with other backends. See #9125. Use `-d:nimLegacyJsRound` for previous behavior.
+
+- Changed the behavior of `uri.decodeQuery` when there are unencoded `=`
+  characters in the decoded values. Prior versions would raise an error. This is
+  no longer the case to comply with the HTML spec and other languages
+  implementations. Old behavior can be obtained with
+  `-d:nimLegacyParseQueryStrict`. `cgi.decodeData` which uses the same
+  underlying code is also updated the same way.
+
+- In `std/os`, `getHomeDir`, `expandTilde`, `getTempDir`, `getConfigDir` now do not include trailing `DirSep`,
+  unless `-d:nimLegacyHomeDir` is specified (for a transition period).
+
 ## Standard library additions and changes
 
 - Added `sections` iterator in `parsecfg`.
@@ -84,9 +118,6 @@
 
 - Added a simpler to use `io.readChars` overload.
 
-- `repr` now doesn't insert trailing newline; previous behavior was very inconsistent,
-  see #16034. Use `-d:nimLegacyReprWithNewline` for previous behavior.
-
 - Added `**` to jsffi.
 
 - `writeStackTrace` is available in JS backend now.
@@ -109,9 +140,6 @@
 
 - Added `math.isNaN`.
 
-- `echo` and `debugEcho` will now raise `IOError` if writing to stdout fails.  Previous behavior
-  silently ignored errors.  See #16366.  Use `-d:nimLegacyEchoNoRaise` for previous behavior.
-
 - Added `jsbigints` module, arbitrary precision integers for JavaScript target.
 
 - Added `math.copySign`.
@@ -130,18 +158,8 @@
 - Added `posix_utils.osReleaseFile` to get system identification from `os-release` file on Linux and the BSDs.
   https://www.freedesktop.org/software/systemd/man/os-release.html
 
-- `math.round` now is rounded "away from zero" in JS backend which is consistent
-  with other backends. See #9125. Use `-d:nimLegacyJsRound` for previous behavior.
-
 - Added `socketstream` module that wraps sockets in the stream interface
 
-- Changed the behavior of `uri.decodeQuery` when there are unencoded `=`
-  characters in the decoded values. Prior versions would raise an error. This is
-  no longer the case to comply with the HTML spec and other languages
-  implementations. Old behavior can be obtained with
-  `-d:nimLegacyParseQueryStrict`. `cgi.decodeData` which uses the same
-  underlying code is also updated the same way.
-
 - Added `sugar.dumpToString` which improves on `sugar.dump`.
 
 - Added `math.signbit`.
@@ -235,7 +253,6 @@
 
 - Added `jscore.debugger` to [call any available debugging functionality, such as breakpoints.](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Statements/debugger).
 
-
 - Added `std/channels`.
 
 - Added `htmlgen.portal` for [making "SPA style" pages using HTML only](https://web.dev/hands-on-portals).
@@ -243,9 +260,6 @@
 - Added `ZZZ` and `ZZZZ` patterns to `times.nim` `DateTime` parsing, to match time
   zone offsets without colons, e.g. `UTC+7 -> +0700`.
 
-- In `std/os`, `getHomeDir`, `expandTilde`, `getTempDir`, `getConfigDir` now do not include trailing `DirSep`,
-  unless `-d:nimLegacyHomeDir` is specified (for a transition period).
-
 - Added `jsconsole.dir`, `jsconsole.dirxml`, `jsconsole.timeStamp`.
 
 - Added dollar `$` and `len` for `jsre.RegExp`.
@@ -254,7 +268,6 @@
 
 - Added `hasClosure` to `std/typetraits`.
 
-
 - Added `genasts.genAst` that avoids the problems inherent with `quote do` and can
   be used as a replacement.
 
@@ -304,9 +317,6 @@
 
 - VM now supports `addr(mystring[ind])` (index + index assignment)
 
-- Type mismatch errors now show more context, use `-d:nimLegacyTypeMismatch` for previous
-  behavior.
-
 - Added `--hintAsError` with similar semantics as `--warningAsError`.
 
 - TLS: OSX now uses native TLS (`--tlsEmulation:off`), TLS now works with importcpp non-POD types,

changelogs/changelog_X_XX_X.md

@@ -3,10 +3,14 @@
 This is an example file.
 The changes should go to changelog.md!
 
+## Changes affecting backward compatibility
+
+- `foo` now behaves differently, use `-d:nimLegacyFoo` for previous behavior.
 
 ## Standard library additions and changes
 
 - Added `example.exampleProc`.
+
 - Changed `example.foo` to take additional `bar` parameter.
 
 

compiler/semexprs.nim

@@ -163,6 +163,11 @@ proc checkConvertible(c: PContext, targetTyp: PType, src: PNode): TConvStatus =
     result = checkConversionBetweenObjects(d.skipTypes(abstractInst), s.skipTypes(abstractInst), pointers)
   elif (targetBaseTyp.kind in IntegralTypes) and
       (srcBaseTyp.kind in IntegralTypes):
+    if targetTyp.kind == tyEnum and srcBaseTyp.kind == tyEnum:
+      if c.config.isDefined("nimLegacyConvEnumEnum"):
+        message(c.config, src.info, warnUser, "enum to enum conversion is now deprecated")
+      else: result = convNotLegal
+    # `elif` would be incorrect here
     if targetTyp.kind == tyBool:
       discard "convOk"
     elif targetTyp.isOrdinalType:

compiler/sempass2.nim

@@ -137,7 +137,7 @@ proc guardGlobal(a: PEffects; n: PNode; guard: PSym) =
   # we allow accesses nevertheless in top level statements for
   # easier initialization:
   #if a.isTopLevel:
-  #  message(n.info, warnUnguardedAccess, renderTree(n))
+  #  message(a.config, n.info, warnUnguardedAccess, renderTree(n))
   #else:
   if not a.isTopLevel:
     localError(a.config, n.info, "unguarded access: " & renderTree(n))
@@ -478,7 +478,7 @@ proc getLockLevel(s: PSym): TLockLevel =
       result = 0.TLockLevel
     else:
       result = UnknownLockLevel
-      #message(s.info, warnUser, "FOR THIS " & s.name.s)
+      #message(??.config, s.info, warnUser, "FOR THIS " & s.name.s)
 
 proc mergeLockLevels(tracked: PEffects, n: PNode, lockLevel: TLockLevel) =
   if lockLevel >= tracked.currLockLevel:
@@ -544,7 +544,7 @@ proc assumeTheWorst(tracked: PEffects; n: PNode; op: PType) =
   let lockLevel = if op.lockLevel == UnspecifiedLockLevel: UnknownLockLevel
                   else: op.lockLevel
   #if lockLevel == UnknownLockLevel:
-  #  message(n.info, warnUser, "had to assume the worst here")
+  #  message(??.config, n.info, warnUser, "had to assume the worst here")
   mergeLockLevels(tracked, n, lockLevel)
 
 proc isOwnedProcVar(n: PNode; owner: PSym): bool =

compiler/vmgen.nim

@@ -1435,7 +1435,7 @@ proc genAddr(c: PCtx, n: PNode, dest: var TDest, flags: TGenFlags) =
       # permanent, so that it's not used for anything else:
       c.prc.slots[tmp].kind = slotTempPerm
       # XXX this is still a hack
-      #message(n.info, warnUser, "suspicious opcode used")
+      #message(c.congig, n.info, warnUser, "suspicious opcode used")
     else:
       gABC(c, n, opcAddrReg, dest, tmp)
     c.freeTemp(tmp)
@@ -1687,7 +1687,7 @@ proc genArrAccessOpcode(c: PCtx; n: PNode; dest: var TDest; opc: TOpcode;
     c.gABC(n, opcNodeToReg, dest, cc)
     c.freeTemp(cc)
   else:
-    #message(n.info, warnUser, "argh")
+    #message(c.config, n.info, warnUser, "argh")
     #echo "FLAGS ", flags, " ", fitsRegister(n.typ), " ", typeToString(n.typ)
     c.gABC(n, opc, dest, a, b)
   c.freeTemp(a)

lib/system/comparisons.nim

@@ -9,7 +9,7 @@ proc `==`*[Enum: enum](x, y: Enum): bool {.magic: "EqEnum", noSideEffect.} =
         place1, place2 = 3
     var
       e1 = field1
-      e2 = Enum1(place2)
+      e2 = place2.ord.Enum1
     assert e1 == e2
     assert not compiles(e1 == place2) # raises error
 proc `==`*(x, y: pointer): bool {.magic: "EqRef", noSideEffect.} =

testament/important_packages.nim

@@ -96,14 +96,15 @@ pkg "nimcrypto", "nim r --path:. tests/testall.nim" # `--path:.` workaround need
 pkg "NimData", "nim c -o:nimdataa src/nimdata.nim"
 pkg "nimes", "nim c src/nimes.nim"
 pkg "nimfp", "nim c -o:nfp -r src/fp.nim"
-pkg "nimgame2", "nim c nimgame2/nimgame.nim", allowFailure = true # XXX Doesn't work with deprecated 'randomize', will create a PR.
+pkg "nimgame2", "nim c -d:nimLegacyConvEnumEnum nimgame2/nimgame.nim", allowFailure = true
+  # XXX Doesn't work with deprecated 'randomize', will create a PR.
 pkg "nimgen", "nim c -o:nimgenn -r src/nimgen/runcfg.nim"
 pkg "nimlsp"
 pkg "nimly", "nim c -r tests/test_readme_example.nim"
 pkg "nimongo", "nimble test_ci", allowFailure = true
 pkg "nimph", "nimble test", "https://github.com/disruptek/nimph", allowFailure = true
 pkg "nimpy", "nim c -r tests/nimfrompy.nim"
-pkg "nimquery"
+pkg "nimquery", allowFailure = true # pending https://github.com/GULPF/nimquery/pull/10
 pkg "nimsl"
 pkg "nimsvg"
 pkg "nimterop", "nimble minitest"

tests/misc/tcast.nim

@@ -48,7 +48,7 @@ block:
   var x: ref int = nil
   doAssert cast[int](cast[ptr int](x)) == 0
 
-block:
+block: # cast of nil
   block:
     static:
       let a = cast[pointer](nil)
@@ -71,7 +71,33 @@ block:
     static:
       doAssert cast[RootRef](nil).repr == "nil"
 
-  # Issue #15730, not fixed yet
-  # block:
-  #   static:
-  #     doAssert cast[cstring](nil).repr == "nil"
+  when false: # xxx bug #15730, not fixed yet
+    block:
+      static:
+        doAssert cast[cstring](nil).repr == "nil"
+
+template main() =
+  # xxx move all under here to get tested in VM
+  block: # cast of enum
+    type Koo = enum k1, k2
+    type Goo = enum g1, g2
+    type Boo = enum b1 = -1, b2, b3, b4
+    type Coo = enum c1 = -1i8, c2, c3, c4
+    when nimvm:
+      # xxx: Error: VM does not support 'cast' from tyEnum to tyEnum
+      discard
+    else:
+      doAssert cast[Koo](k2) == k2
+      doAssert cast[Goo](k2) == g2
+      doAssert cast[Goo](k2.ord) == g2
+
+      doAssert b3.ord == 1
+      doAssert cast[Koo](b3) == k2
+      doAssert cast[Boo](k2) == b3
+
+      doAssert c3.ord == 1
+      doAssert cast[Koo](c3) == k2
+      doAssert cast[Coo](k2) == c3
+
+static: main()
+main()

tests/misc/tconv.nim

@@ -1,5 +1,13 @@
+discard """
+  nimout:'''
+tconv.nim(81, 15) Warning: enum to enum conversion is now deprecated [User]
+'''
+"""
+
 template reject(x) =
-    static: assert(not compiles(x))
+  static: doAssert(not compiles(x))
+template accept(x) =
+  static: doAssert(compiles(x))
 
 reject:
     const x = int8(300)
@@ -55,3 +63,26 @@ block: # issue 3766
     proc r(x: static[R]) =
       echo x
     r 3.R
+
+
+block: # https://github.com/nim-lang/RFCs/issues/294
+  type Koo = enum k1, k2
+  type Goo = enum g1, g2
+
+  accept: Koo(k2)
+  accept: k2.Koo
+  accept: k2.int.Goo
+
+  reject: Goo(k2)
+  reject: k2.Goo
+  reject: k2.string
+
+  {.define(nimLegacyConvEnumEnum).}
+  discard Goo(k2)
+  accept: Goo(k2)
+  accept: k2.Goo
+  reject: k2.string
+  {.undef(nimLegacyConvEnumEnum).}
+
+  reject: Goo(k2)
+  reject: k2.Goo