From 270964c487e5347c61dade25bec903580483dda5 Mon Sep 17 00:00:00 2001 From: Timothee Cour Date: Fri, 2 Apr 2021 22:15:21 -0700 Subject: [PATCH] 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 | 58 +++++++++++++++++++------------- changelogs/changelog_X_XX_X.md | 4 +++ compiler/semexprs.nim | 5 +++ compiler/sempass2.nim | 6 ++-- compiler/vmgen.nim | 4 +-- lib/system/comparisons.nim | 2 +- testament/important_packages.nim | 5 +-- tests/misc/tcast.nim | 36 +++++++++++++++++--- tests/misc/tconv.nim | 33 +++++++++++++++++- 9 files changed, 115 insertions(+), 38 deletions(-) diff --git a/changelog.md b/changelog.md index 020d8a350a..43a15e398b 100644 --- a/changelog.md +++ b/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, diff --git a/changelogs/changelog_X_XX_X.md b/changelogs/changelog_X_XX_X.md index 48520fd44c..77b421f33e 100644 --- a/changelogs/changelog_X_XX_X.md +++ b/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. diff --git a/compiler/semexprs.nim b/compiler/semexprs.nim index 27b78aa6f5..71c9e0295e 100644 --- a/compiler/semexprs.nim +++ b/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: diff --git a/compiler/sempass2.nim b/compiler/sempass2.nim index 87d196fad3..52998c2149 100644 --- a/compiler/sempass2.nim +++ b/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 = diff --git a/compiler/vmgen.nim b/compiler/vmgen.nim index be26f98b3c..69d7ec4b37 100644 --- a/compiler/vmgen.nim +++ b/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) diff --git a/lib/system/comparisons.nim b/lib/system/comparisons.nim index 2e5664a958..7122daa209 100644 --- a/lib/system/comparisons.nim +++ b/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.} = diff --git a/testament/important_packages.nim b/testament/important_packages.nim index 8a8d6ffd08..8f096b71e5 100644 --- a/testament/important_packages.nim +++ b/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" diff --git a/tests/misc/tcast.nim b/tests/misc/tcast.nim index 454801a2d0..6d67b1c528 100644 --- a/tests/misc/tcast.nim +++ b/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() diff --git a/tests/misc/tconv.nim b/tests/misc/tconv.nim index f7d15b0b57..c93fc57f84 100644 --- a/tests/misc/tconv.nim +++ b/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