From 6c5872c1690dcd5f50477aeef0218a0dbbcdd6e6 Mon Sep 17 00:00:00 2001
From: Timothee Cour <timothee.cour2@gmail.com>
Date: Mon, 1 Mar 2021 11:59:43 -0800
Subject: [PATCH] `--nilseqs` is now a deprecated noop (#17211)

* --nilseqs is now a deprecated noop

* fix tests; fix: future => sugar
---
 changelog.md                                  |  2 ++
 compiler/ccgexprs.nim                         | 16 ++++-----
 compiler/commands.nim                         |  6 ++--
 compiler/msgs.nim                             |  3 ++
 compiler/options.nim                          |  1 -
 compiler/sigmatch.nim                         | 10 ++----
 doc/advopt.txt                                |  2 --
 lib/system.nim                                | 33 ++++++++-----------
 testament/categories.nim                      |  2 --
 tests/ccgbugs/tassign_nil_strings.nim         |  2 +-
 tests/cpp/tasync_cpp.nim                      |  2 +-
 .../keineschweine/keineschweine.nim.cfg       |  1 -
 tests/niminaction/Chapter1/various1.nim       |  2 +-
 tests/niminaction/Chapter2/resultaccept.nim   |  2 +-
 tests/niminaction/Chapter2/various2.nim       |  6 ++--
 15 files changed, 37 insertions(+), 53 deletions(-)

diff --git a/changelog.md b/changelog.md
index a21b23b5d4..771a08ba60 100644
--- a/changelog.md
+++ b/changelog.md
@@ -214,6 +214,8 @@ provided by the operating system.
 
 - Deprecated `TaintedString` and `--taintmode`.
 
+- Deprecated `--nilseqs` which is now a noop.
+
 - Source+Edit links now appear on top of every docgen'd page when
   `nim doc --git.url:url ...` is given.
 
diff --git a/compiler/ccgexprs.nim b/compiler/ccgexprs.nim
index e3cc93d4be..86e0ed8c01 100644
--- a/compiler/ccgexprs.nim
+++ b/compiler/ccgexprs.nim
@@ -85,10 +85,9 @@ proc genLiteral(p: BProc, n: PNode, ty: PType): Rope =
     of tyNil:
       result = genNilStringLiteral(p.module, n.info)
     of tyString:
-      # with the new semantics for 'nil' strings, we can map "" to nil and
+      # with the new semantics for not 'nil' strings, we can map "" to nil and
       # save tons of allocations:
-      if n.strVal.len == 0 and optNilSeqs notin p.options and
-          optSeqDestructors notin p.config.globalOptions:
+      if n.strVal.len == 0 and optSeqDestructors notin p.config.globalOptions:
         result = genNilStringLiteral(p.module, n.info)
       else:
         result = genStringLiteral(p.module, n)
@@ -176,8 +175,7 @@ proc canMove(p: BProc, n: PNode; dest: TLoc): bool =
     if not isDeepConstExpr(n) or n.len == 0:
       if skipTypes(n.typ, abstractVarRange).kind == tySequence:
         return true
-  elif optNilSeqs notin p.options and
-    n.kind in nkStrKinds and n.strVal.len == 0:
+  elif n.kind in nkStrKinds and n.strVal.len == 0:
     # Empty strings are codegen'd as NIM_NIL so it's just a pointer copy
     return true
   result = n.kind in nkCallKinds
@@ -1371,8 +1369,7 @@ proc genNewSeq(p: BProc, e: PNode) =
        getTypeDesc(p.module, seqtype.lastSon),
        getSeqPayloadType(p.module, seqtype)])
   else:
-    let lenIsZero = optNilSeqs notin p.options and
-      e[2].kind == nkIntLit and e[2].intVal == 0
+    let lenIsZero = e[2].kind == nkIntLit and e[2].intVal == 0
     genNewSeqAux(p, a, b.rdLoc, lenIsZero)
     gcUsage(p.config, e)
 
@@ -1497,8 +1494,7 @@ proc genSeqConstr(p: BProc, n: PNode, d: var TLoc) =
       getSeqPayloadType(p.module, seqtype)])
   else:
     # generate call to newSeq before adding the elements per hand:
-    genNewSeqAux(p, dest[], l,
-      optNilSeqs notin p.options and n.len == 0)
+    genNewSeqAux(p, dest[], l, n.len == 0)
   for i in 0..<n.len:
     initLoc(arr, locExpr, n[i], OnHeap)
     arr.r = ropecg(p.module, "$1$3[$2]", [rdLoc(dest[]), intLiteral(i), dataField(p)])
@@ -1527,7 +1523,7 @@ proc genArrToSeq(p: BProc, n: PNode, d: var TLoc) =
       [rdLoc d, L, getTypeDesc(p.module, seqtype.lastSon),
       getSeqPayloadType(p.module, seqtype)])
   else:
-    genNewSeqAux(p, d, intLiteral(L), optNilSeqs notin p.options and L == 0)
+    genNewSeqAux(p, d, intLiteral(L), L == 0)
   initLocExpr(p, n[1], a)
   # bug #5007; do not produce excessive C source code:
   if L < 10:
diff --git a/compiler/commands.nim b/compiler/commands.nim
index fa53a3ce66..f133d5adf7 100644
--- a/compiler/commands.nim
+++ b/compiler/commands.nim
@@ -318,7 +318,9 @@ proc testCompileOption*(conf: ConfigRef; switch: string, info: TLineInfo): bool
   of "implicitstatic": result = contains(conf.options, optImplicitStatic)
   of "patterns", "trmacros": result = contains(conf.options, optTrMacros)
   of "excessivestacktrace": result = contains(conf.globalOptions, optExcessiveStackTrace)
-  of "nilseqs": result = contains(conf.options, optNilSeqs)
+  of "nilseqs":
+    warningDeprecated(conf, info, "nilseqs is a deprecated noop")
+    result = false
   else: invalidCmdLineOption(conf, passCmd1, switch, info)
 
 proc processPath(conf: ConfigRef; path: string, info: TLineInfo,
@@ -650,7 +652,7 @@ proc processSwitch*(switch, arg: string, pass: TCmdLinePass, info: TLineInfo;
     else:
       undefSymbol(conf.symbols, "hotcodereloading")
       undefSymbol(conf.symbols, "useNimRtl")
-  of "nilseqs": processOnOffSwitch(conf, {optNilSeqs}, arg, pass, info)
+  of "nilseqs": warningDeprecated(conf, info, "nilseqs is a deprecated noop")
   of "checks", "x": processOnOffSwitch(conf, ChecksOptions, arg, pass, info)
   of "floatchecks":
     processOnOffSwitch(conf, {optNaNCheck, optInfCheck}, arg, pass, info)
diff --git a/compiler/msgs.nim b/compiler/msgs.nim
index e796b18ee1..b384dad243 100644
--- a/compiler/msgs.nim
+++ b/compiler/msgs.nim
@@ -590,6 +590,9 @@ template localError*(conf: ConfigRef; info: TLineInfo, format: string, params: o
 template message*(conf: ConfigRef; info: TLineInfo, msg: TMsgKind, arg = "") =
   liMessage(conf, info, msg, arg, doNothing, instLoc())
 
+proc warningDeprecated*(conf: ConfigRef, info: TLineInfo = gCmdLineInfo, msg = "") {.inline.} =
+  message(conf, info, warnDeprecated, msg)
+
 proc internalErrorImpl(conf: ConfigRef; info: TLineInfo, errMsg: string, info2: InstantiationInfo) =
   if conf.cmd == cmdIdeTools and conf.structuredErrorHook.isNil: return
   writeContext(conf, info)
diff --git a/compiler/options.nim b/compiler/options.nim
index 4cb0f55a30..3d46b6ad1c 100644
--- a/compiler/options.nim
+++ b/compiler/options.nim
@@ -39,7 +39,6 @@ type                          # please make sure we have under 32 options
                               # evaluation
     optTrMacros,              # en/disable pattern matching
     optMemTracker,
-    optNilSeqs,
     optSinkInference          # 'sink T' inference
     optCursorInference
 
diff --git a/compiler/sigmatch.nim b/compiler/sigmatch.nim
index 26124e81ce..87f7c273b1 100644
--- a/compiler/sigmatch.nim
+++ b/compiler/sigmatch.nim
@@ -556,12 +556,6 @@ proc recordRel(c: var TCandidate, f, a: PType): TTypeRelation =
 proc allowsNil(f: PType): TTypeRelation {.inline.} =
   result = if tfNotNil notin f.flags: isSubtype else: isNone
 
-proc allowsNilDeprecated(c: TCandidate, f: PType): TTypeRelation =
-  if optNilSeqs in c.c.config.options:
-    result = allowsNil(f)
-  else:
-    result = isNone
-
 proc inconsistentVarTypes(f, a: PType): bool {.inline.} =
   result = f.kind != a.kind and
     (f.kind in {tyVar, tyLent, tySink} or a.kind in {tyVar, tyLent, tySink})
@@ -1300,7 +1294,7 @@ proc typeRel(c: var TCandidate, f, aOrig: PType,
             result = isNone
         elif tfNotNil in f.flags and tfNotNil notin a.flags:
           result = isNilConversion
-    of tyNil: result = allowsNilDeprecated(c, f)
+    of tyNil: result = isNone
     else: discard
   of tyOrdinal:
     if isOrdinalType(a, allowEnumWithHoles = optNimV1Emulation in c.c.config.globalOptions):
@@ -1396,7 +1390,7 @@ proc typeRel(c: var TCandidate, f, aOrig: PType,
         result = isNilConversion
       else:
         result = isEqual
-    of tyNil: result = allowsNilDeprecated(c, f)
+    of tyNil: result = isNone
     else: discard
   of tyCString:
     # conversion from string to cstring is automatic:
diff --git a/doc/advopt.txt b/doc/advopt.txt
index 7aa5d3a94e..a992079d9d 100644
--- a/doc/advopt.txt
+++ b/doc/advopt.txt
@@ -102,8 +102,6 @@ Advanced options:
   --excessiveStackTrace:on|off
                             stack traces use full file paths
   --stackTraceMsgs:on|off   enable user defined stack frame msgs via `setFrameMsg`
-  --nilseqs:on|off          allow 'nil' for strings/seqs for
-                            backwards compatibility
   --seqsv2:on|off           use the new string/seq implementation based on
                             destructors
   --skipCfg:on|off          do not read the nim installation's configuration file
diff --git a/lib/system.nim b/lib/system.nim
index 6b7549a481..56746fb659 100644
--- a/lib/system.nim
+++ b/lib/system.nim
@@ -1600,26 +1600,19 @@ proc len*[U: Ordinal; V: Ordinal](x: HSlice[U, V]): int {.noSideEffect, inline.}
   ##   assert((5..2).len == 0)
   result = max(0, ord(x.b) - ord(x.a) + 1)
 
-when not compileOption("nilseqs"):
-  {.pragma: nilError, error.}
-else:
-  {.pragma: nilError.}
+when true: # PRTEMP: remove?
+  proc isNil*[T](x: seq[T]): bool {.noSideEffect, magic: "IsNil", error.}
+    ## Seqs are no longer nil by default, but set and empty.
+    ## Check for zero length instead.
+    ##
+    ## See also:
+    ## * `isNil(string) <#isNil,string>`_
 
-proc isNil*[T](x: seq[T]): bool {.noSideEffect, magic: "IsNil", nilError.}
-  ## Requires `--nilseqs:on` since 0.19.
-  ##
-  ## Seqs are no longer nil by default, but set and empty.
-  ## Check for zero length instead.
-  ##
-  ## See also:
-  ## * `isNil(string) <#isNil,string>`_
+  proc isNil*(x: string): bool {.noSideEffect, magic: "IsNil", error.}
+    ## See also:
+    ## * `isNil(seq[T]) <#isNil,seq[T]>`_
 
 proc isNil*[T](x: ref T): bool {.noSideEffect, magic: "IsNil".}
-proc isNil*(x: string): bool {.noSideEffect, magic: "IsNil", nilError.}
-  ## Requires `--nilseqs:on`.
-  ##
-  ## See also:
-  ## * `isNil(seq[T]) <#isNil,seq[T]>`_
 
 proc isNil*[T](x: ptr T): bool {.noSideEffect, magic: "IsNil".}
 proc isNil*(x: pointer): bool {.noSideEffect, magic: "IsNil".}
@@ -2985,15 +2978,15 @@ proc `==`*(x, y: cstring): bool {.magic: "EqCString", noSideEffect,
   elif x.isNil or y.isNil: result = false
   else: result = strcmp(x, y) == 0
 
-when not compileOption("nilseqs"):
+when true: # xxx PRTEMP remove
   # bug #9149; ensure that 'typeof(nil)' does not match *too* well by using 'typeof(nil) | typeof(nil)',
   # especially for converters, see tests/overload/tconverter_to_string.nim
   # Eventually we will be able to remove this hack completely.
   proc `==`*(x: string; y: typeof(nil) | typeof(nil)): bool {.
-      error: "'nil' is now invalid for 'string'; compile with --nilseqs:on for a migration period".} =
+      error: "'nil' is now invalid for 'string'".} =
     discard
   proc `==`*(x: typeof(nil) | typeof(nil); y: string): bool {.
-      error: "'nil' is now invalid for 'string'; compile with --nilseqs:on for a migration period".} =
+      error: "'nil' is now invalid for 'string'".} =
     discard
 
 template closureScope*(body: untyped): untyped =
diff --git a/testament/categories.nim b/testament/categories.nim
index e6f9698df2..96449a0239 100644
--- a/testament/categories.nim
+++ b/testament/categories.nim
@@ -250,8 +250,6 @@ proc jsTests(r: var TResults, cat: Category, options: string) =
 # ------------------------- nim in action -----------
 
 proc testNimInAction(r: var TResults, cat: Category, options: string) =
-  let options = options & " --nilseqs:on"
-
   template test(filename: untyped) =
     testSpec r, makeTest(filename, options, cat)
 
diff --git a/tests/ccgbugs/tassign_nil_strings.nim b/tests/ccgbugs/tassign_nil_strings.nim
index 07d2c2aeb9..f6fab7baac 100644
--- a/tests/ccgbugs/tassign_nil_strings.nim
+++ b/tests/ccgbugs/tassign_nil_strings.nim
@@ -1,5 +1,5 @@
 discard """
-  cmd: "nim $target --nilseqs:off $options $file"
+  cmd: "nim $target $options $file"
   output: "Hello"
   ccodecheck: "\\i@'a = ((NimStringDesc*) NIM_NIL)'"
 """
diff --git a/tests/cpp/tasync_cpp.nim b/tests/cpp/tasync_cpp.nim
index bc5d48eb5b..696274ec47 100644
--- a/tests/cpp/tasync_cpp.nim
+++ b/tests/cpp/tasync_cpp.nim
@@ -1,7 +1,7 @@
 discard """
   targets: "cpp"
   output: "hello"
-  cmd: "nim cpp --nilseqs:on --clearNimblePath --nimblePath:build/deps/pkgs $file"
+  cmd: "nim cpp --clearNimblePath --nimblePath:build/deps/pkgs $file"
 """
 
 # bug #3299
diff --git a/tests/manyloc/keineschweine/keineschweine.nim.cfg b/tests/manyloc/keineschweine/keineschweine.nim.cfg
index f335a0e0e3..ca6c75f6ed 100644
--- a/tests/manyloc/keineschweine/keineschweine.nim.cfg
+++ b/tests/manyloc/keineschweine/keineschweine.nim.cfg
@@ -7,4 +7,3 @@ path = "dependencies/genpacket"
 path = "enet_server"
 debugger = off
 warning[SmallLshouldNotBeUsed] = off
-nilseqs = on
diff --git a/tests/niminaction/Chapter1/various1.nim b/tests/niminaction/Chapter1/various1.nim
index 4e2cb463d8..21553dc40e 100644
--- a/tests/niminaction/Chapter1/various1.nim
+++ b/tests/niminaction/Chapter1/various1.nim
@@ -32,7 +32,7 @@ block: # Block added due to clash.
   let dog = Dog()
   dog.bark() #<2>
 
-import sequtils, future, strutils
+import sequtils, sugar, strutils
 let list = @["Dominik Picheta", "Andreas Rumpf", "Desmond Hume"]
 list.map(
   (x: string) -> (string, string) => (x.split[0], x.split[1])
diff --git a/tests/niminaction/Chapter2/resultaccept.nim b/tests/niminaction/Chapter2/resultaccept.nim
index 7dd976b40e..390f7b3299 100644
--- a/tests/niminaction/Chapter2/resultaccept.nim
+++ b/tests/niminaction/Chapter2/resultaccept.nim
@@ -22,7 +22,7 @@ proc resultVar2: string =
   result.add("returned")
 
 doAssert implicit() == "I will be returned"
-doAssert discarded() == nil
+doAssert discarded().len == 0
 doAssert explicit() == "I will be returned"
 doAssert resultVar() == "I will be returned"
 doAssert resultVar2() == "I will be returned"
\ No newline at end of file
diff --git a/tests/niminaction/Chapter2/various2.nim b/tests/niminaction/Chapter2/various2.nim
index 488c361a28..921f38c7d1 100644
--- a/tests/niminaction/Chapter2/various2.nim
+++ b/tests/niminaction/Chapter2/various2.nim
@@ -140,7 +140,7 @@ let numbers = @[1, 2, 3, 4, 5, 6]
 let odd = filter(numbers, proc (x: int): bool = x mod 2 != 0)
 doAssert odd == @[1, 3, 5]
 
-import sequtils, future
+import sequtils, sugar
 let numbers1 = @[1, 2, 3, 4, 5, 6]
 let odd1 = filter(numbers1, (x: int) -> bool => x mod 2 != 0)
 assert odd1 == @[1, 3, 5]
@@ -149,7 +149,7 @@ proc isValid(x: int, validator: proc (x: int): bool) =
   if validator(x): echo(x, " is valid")
   else: echo(x, " is NOT valid")
 
-import future
+import sugar
 proc isValid2(x: int, validator: (x: int) -> bool) =
   if validator(x): echo(x, " is valid")
   else: echo(x, " is NOT valid")
@@ -193,7 +193,7 @@ doAssertRaises(IndexDefect):
 
 block:
   var list = newSeq[string](3)
-  assert list[0] == nil
+  assert list[0].len == 0
   list[0] = "Foo"
   list[1] = "Bar"
   list[2] = "Baz"