deprecate do: meaning do (): + misc cleanup (#20927)

* test disable do: block lambda lifting

* fix last test [skip ci]

* deprecate `do:` meaning `do ():` + misc cleanup

closes https://github.com/nim-lang/RFCs/issues/486

* oops

* fix

* no idea what could be causing nimsuggest failure other than this

* ensure ci works

changelog.md

@@ -107,8 +107,17 @@
 
 - Object fields now support default values, see https://nim-lang.github.io/Nim/manual.html#types-default-values-for-object-fields for details.
 
+- Redefining templates with the same signature was previously
+  allowed to support certain macro code. To do this explicitly, the
+  `{.redefine.}` pragma has been added. Note that this is only for templates.
+  Implicit redefinition of templates is now deprecated and will give an error in the future.
+
 - Using an unnamed break in a block is deprecated. This warning will become an error in future versions! Use a named block with a named break instead.
 
+- Previously, calls like `foo(a, b): ...` or `foo(a, b) do: ...` where the final argument of
+  `foo` had type `proc ()` were assumed by the compiler to mean `foo(a, b, proc () = ...)`.
+  This behavior is now deprecated. Use `foo(a, b) do (): ...` or `foo(a, b, proc () = ...)` instead.
+
 ## Standard library additions and changes
 
 [//]: # "Changes:"
@@ -229,12 +238,6 @@
       Baz = object
     ```
 
-- Redefining templates with the same signature implicitly was previously
-  allowed to support certain macro code. A `{.redefine.}` pragma has been
-  added to make this work explicitly, and a warning is generated in the case
-  where it is implicit. This behavior only applies to templates, redefinition
-  is generally disallowed for other symbols.
-
 - A new form of type inference called [top-down inference](https://nim-lang.github.io/Nim/manual_experimental.html#topminusdown-type-inference)
   has been implemented for a variety of basic cases. For example, code like the following now compiles:
 

compiler/lineinfos.nim

@@ -83,8 +83,9 @@ type
     warnPtrToCstringConv = "PtrToCstringConv",
     warnEffect = "Effect",
     warnCastSizes = "CastSizes"
-    warnTemplateRedefinition = "TemplateRedefinition",
+    warnImplicitTemplateRedefinition = "ImplicitTemplateRedefinition",
     warnUnnamedBreak = "UnnamedBreak",
+    warnStmtListLambda = "StmtListLambda",
     warnUser = "User",
     # hints
     hintSuccess = "Success", hintSuccessX = "SuccessX",
@@ -181,8 +182,9 @@ const
     warnPtrToCstringConv: "unsafe conversion to 'cstring' from '$1'; this will become a compile time error in the future",
     warnEffect: "$1",
     warnCastSizes: "$1",
-    warnTemplateRedefinition: "template '$1' is implicitly redefined, consider adding an explicit .redefine pragma",
+    warnImplicitTemplateRedefinition: "template '$1' is implicitly redefined; this is deprecated, add an explicit .redefine pragma",
     warnUnnamedBreak: "Using an unnamed break in a block is deprecated; Use a named block with a named break instead",
+    warnStmtListLambda: "statement list expression assumed to be anonymous proc; this is deprecated, use `do (): ...` or `proc () = ...` instead",
     warnUser: "$1",
     hintSuccess: "operation successful: $#",
     # keep in sync with `testament.isSuccess`

compiler/semexprs.nim

@@ -1266,15 +1266,7 @@ proc semSym(c: PContext, n: PNode, sym: PSym, flags: TExprFlags): PNode =
         result = newSymNode(s, n.info)
     else:
       result = newSymNode(s, n.info)
-  of skMacro:
+  of skMacro, skTemplate:
-    if efNoEvaluateGeneric in flags and s.ast[genericParamsPos].len > 0 or
-       (n.kind notin nkCallKinds and s.requiredParams > 0):
-      markUsed(c, n.info, s)
-      onUse(n.info, s)
-      result = symChoice(c, n, s, scClosed)
-    else:
-      result = semMacroExpr(c, n, n, s, flags)
-  of skTemplate:
     if efNoEvaluateGeneric in flags and s.ast[genericParamsPos].len > 0 or
        (n.kind notin nkCallKinds and s.requiredParams > 0) or
        sfCustomPragma in sym.flags:
@@ -1283,7 +1275,10 @@ proc semSym(c: PContext, n: PNode, sym: PSym, flags: TExprFlags): PNode =
       onUse(info, s)
       result = symChoice(c, n, s, scClosed)
     else:
-      result = semTemplateExpr(c, n, s, flags)
+      case s.kind
+      of skMacro: result = semMacroExpr(c, n, n, s, flags)
+      of skTemplate: result = semTemplateExpr(c, n, s, flags)
+      else: discard # unreachable
   of skParam:
     markUsed(c, n.info, s)
     onUse(n.info, s)

compiler/semstmts.nim

@@ -509,13 +509,6 @@ proc semVarMacroPragma(c: PContext, a: PNode, n: PNode): PNode =
       let it = pragmas[i]
       let key = if it.kind in nkPragmaCallKinds and it.len >= 1: it[0] else: it
 
-      when false:
-        let lhs = b[0]
-        let clash = strTableGet(c.currentScope.symbols, lhs.ident)
-        if clash != nil:
-          # refs https://github.com/nim-lang/Nim/issues/8275
-          wrongRedefinition(c, lhs.info, lhs.ident.s, clash.info)
-
       if isPossibleMacroPragma(c, it, key):
         # we transform ``var p {.m, rest.}`` into ``m(do: var p {.rest.})`` and
         # let the semantic checker deal with it:
@@ -562,27 +555,6 @@ proc semVarMacroPragma(c: PContext, a: PNode, n: PNode): PNode =
 
         doAssert result != nil
 
-        # since a macro pragma can set pragmas, we process these here again.
-        # This is required for SqueakNim-like export pragmas.
-        if false and result.kind in {nkVarSection, nkLetSection, nkConstSection}:
-          var validPragmas: TSpecialWords
-          case result.kind
-          of nkVarSection:
-            validPragmas = varPragmas
-          of nkLetSection:
-            validPragmas = letPragmas
-          of nkConstSection:
-            validPragmas = constPragmas
-          else:
-            # unreachable
-            discard
-          for defs in result:
-            for i in 0 ..< defs.len - 2:
-              let ex = defs[i]
-              if ex.kind == nkPragmaExpr and
-                  ex[namePos].kind == nkSym and
-                  ex[pragmaPos].kind != nkEmpty:
-                pragma(c, defs[lhsPos][namePos].sym, defs[lhsPos][pragmaPos], validPragmas)
         return result
 
 template isLocalVarSym(n: PNode): bool =
@@ -1710,12 +1682,6 @@ proc semProcAnnotation(c: PContext, prc: PNode;
 
       doAssert result != nil
 
-      # since a proc annotation can set pragmas, we process these here again.
-      # This is required for SqueakNim-like export pragmas.
-      if false and result.kind in procDefs and result[namePos].kind == nkSym and
-          result[pragmasPos].kind != nkEmpty:
-        pragma(c, result[namePos].sym, result[pragmasPos], validPragmas)
-
       return result
 
 proc semInferredLambda(c: PContext, pt: TIdTable, n: PNode): PNode {.nosinks.} =

compiler/semtempl.nim

@@ -691,7 +691,7 @@ proc semTemplateDef(c: PContext, n: PNode): PNode =
   elif not comesFromShadowscope:
     if {sfTemplateRedefinition, sfGenSym} * s.flags == {}:
       #wrongRedefinition(c, n.info, proto.name.s, proto.info)
-      message(c.config, n.info, warnTemplateRedefinition, s.name.s)
+      message(c.config, n.info, warnImplicitTemplateRedefinition, s.name.s)
     symTabReplace(c.currentScope.symbols, proto, s)
   if n[patternPos].kind != nkEmpty:
     c.patterns.add(s)

compiler/sigmatch.nim

@@ -2189,6 +2189,8 @@ proc paramTypesMatchAux(m: var TCandidate, f, a: PType,
       return arg
     elif a.kind == tyVoid and f.matchesVoidProc and argOrig.kind == nkStmtList:
       # lift do blocks without params to lambdas
+      # now deprecated
+      message(c.config, argOrig.info, warnStmtListLambda)
       let p = c.graph
       let lifted = c.semExpr(c, newProcNode(nkDo, argOrig.info, body = argOrig,
           params = nkFormalParams.newTree(p.emptyNode), name = p.emptyNode, pattern = p.emptyNode,

tests/closure/tclosure.nim

@@ -239,19 +239,19 @@ block doNotation:
   b.onClick do (e: Event):
     echo "click at ", e.x, ",", e.y
 
-  b.onFocusLost:
+  b.onFocusLost do ():
     echo "lost focus 1"
 
-  b.onFocusLost do:
+  b.onFocusLost do ():
     echo "lost focus 2"
 
-  b.onUserEvent("UserEvent 1") do:
+  b.onUserEvent("UserEvent 1") do ():
     discard
 
-  b.onUserEvent "UserEvent 2":
+  onUserEvent(b, "UserEvent 2") do ():
     discard
 
-  b.onUserEvent("UserEvent 3"):
+  b.onUserEvent("UserEvent 3") do ():
     discard
 
   b.onUserEvent("UserEvent 4", () => echo "event 4")

tests/closure/tclosure_issues.nim

@@ -71,12 +71,12 @@ block tissue7104:
   proc sp(cb: proc())=
       cb()
 
-  sp:
+  sp do ():
       var i = 0
       echo "ok ", i
-      sp():
+      sp do ():
           inc i
           echo "ok ", i
-          sp do:
+          sp do ():
               inc i
               echo "ok ", i

tests/closure/tstmtlist.nim

@@ -0,0 +1,9 @@
+discard """
+  action: compile
+"""
+
+proc foo(x: proc()) = x()
+foo: echo "a" #[tt.Warning
+     ^ statement list expression assumed to be anonymous proc; this is deprecated, use `do (): ...` or `proc () = ...` instead [StmtListLambda]]#
+foo do: echo "b" #[tt.Warning
+        ^ statement list expression assumed to be anonymous proc; this is deprecated, use `do (): ...` or `proc () = ...` instead [StmtListLambda]]#

tests/js/tjsffi.nim

@@ -217,7 +217,7 @@ block:
   on("click") do (e: Event):
     console.log e
 
-  jslib.on("reloaded") do:
+  jslib.on("reloaded") do ():
     console.log jsarguments[0]
 
   # this test case is different from the above, because

tests/js/tjsffi_old.nim

@@ -321,7 +321,7 @@ block:
   on("click") do (e: Event):
     console.log e
 
-  jslib.on("reloaded") do:
+  jslib.on("reloaded") do ():
     console.log jsarguments[0]
 
   # this test case is different from the above, because

tests/misc/tlambdadonotation.nim

@@ -67,7 +67,7 @@ proc main2() =
   proc foo() =
     subscriber.consume()
 
-  emitter.on_event() do:
+  emitter.on_event() do ():
     subscriber.consume()
 
   # this works

tests/template/tredefinition_override.nim

@@ -1,4 +1,4 @@
-{.push warningAsError[TemplateRedefinition]: on.}
+{.push warningAsError[ImplicitTemplateRedefinition]: on.}
 
 doAssert not (compiles do:
   template foo(): int = 1