From 848a9ac98ff0ea88b9ade053eacf57a4687109a0 Mon Sep 17 00:00:00 2001
From: Timothee Cour <timothee.cour2@gmail.com>
Date: Wed, 24 Jul 2019 18:24:36 -0700
Subject: [PATCH 01/14] nim now has ref-alias and ptr-alias for lvalue
 expressions

---
 lib/core/macros.nim | 40 ++++++++++++++++++++++++++++++++++++++++
 1 file changed, 40 insertions(+)

diff --git a/lib/core/macros.nim b/lib/core/macros.nim
index f97b4a15fb174..812e6b3d53728 100644
--- a/lib/core/macros.nim
+++ b/lib/core/macros.nim
@@ -1677,3 +1677,43 @@ when defined(nimMacrosSizealignof):
 
 proc isExported*(n: NimNode): bool {.noSideEffect.} =
   ## Returns whether the symbol is exported or not.
+
+proc splitDefinition*(def: NimNode): tuple[lhs: NimNode, rhs: NimNode] =
+  ## allows library constructs such as: `byRef: a2 = expr`
+  doAssert def.kind == nnkStmtList and def.len == 1
+  let def2 = def[0]
+  doAssert def2.kind == nnkAsgn
+  let lhs = def2[0]
+  let rhs = def2[1]
+  expectKind(lhs, nnkIdent)
+  return (lhs, rhs)
+
+macro byRef*(def: untyped): untyped =
+  ## Defines a ref alias for lvalue expressions. The expression is evaluated
+  ## only once, and any side effects will only be evaluated once, at declaration
+  ## time.
+  runnableExamples:
+    var count = 0
+    proc identity(a: int): auto =
+      block: count.inc; a
+    var x = @[1,2,3]
+    byRef: x1=x[identity(1)] # the ref-alias is evaluated only here
+    doAssert count == 1 
+    x1 += 10
+    doAssert type(x1) is int # use x1 just like a normal variable
+    doAssert x == @[1,12,3]
+    doAssert count == 1 # count has not changed
+
+  let (name, exp) = splitDefinition(def)
+  result = quote do:
+    let myAddr = addr `exp`
+    template `name`: untyped = myAddr[]
+
+macro byPtr*(def: untyped): untyped =
+  ## Defines a ptr alias for expressions. Caution: this uses `unsafeAddr`, so
+  ## is unsafe to use in general, and `byRef` should be preferred when possible.
+  ## This can for example be used on `let` variables.
+  let (name, exp) = splitDefinition(def)
+  result = quote do:
+    let myAddr = unsafeAddr `exp`
+    template `name`: untyped = myAddr[]

From 774ff5d0d33d6af06448f22ce6b2c19db52c6a23 Mon Sep 17 00:00:00 2001
From: Timothee Cour <timothee.cour2@gmail.com>
Date: Thu, 25 Jul 2019 15:17:31 -0700
Subject: [PATCH 02/14] address comments

---
 lib/core/macros.nim     | 40 --------------------------
 lib/pure/sugar.nim      | 34 ++++++++++++++++++++++
 tests/stdlib/tsugar.nim | 63 +++++++++++++++++++++++++++++++++++++++++
 3 files changed, 97 insertions(+), 40 deletions(-)
 create mode 100644 tests/stdlib/tsugar.nim

diff --git a/lib/core/macros.nim b/lib/core/macros.nim
index 812e6b3d53728..f97b4a15fb174 100644
--- a/lib/core/macros.nim
+++ b/lib/core/macros.nim
@@ -1677,43 +1677,3 @@ when defined(nimMacrosSizealignof):
 
 proc isExported*(n: NimNode): bool {.noSideEffect.} =
   ## Returns whether the symbol is exported or not.
-
-proc splitDefinition*(def: NimNode): tuple[lhs: NimNode, rhs: NimNode] =
-  ## allows library constructs such as: `byRef: a2 = expr`
-  doAssert def.kind == nnkStmtList and def.len == 1
-  let def2 = def[0]
-  doAssert def2.kind == nnkAsgn
-  let lhs = def2[0]
-  let rhs = def2[1]
-  expectKind(lhs, nnkIdent)
-  return (lhs, rhs)
-
-macro byRef*(def: untyped): untyped =
-  ## Defines a ref alias for lvalue expressions. The expression is evaluated
-  ## only once, and any side effects will only be evaluated once, at declaration
-  ## time.
-  runnableExamples:
-    var count = 0
-    proc identity(a: int): auto =
-      block: count.inc; a
-    var x = @[1,2,3]
-    byRef: x1=x[identity(1)] # the ref-alias is evaluated only here
-    doAssert count == 1 
-    x1 += 10
-    doAssert type(x1) is int # use x1 just like a normal variable
-    doAssert x == @[1,12,3]
-    doAssert count == 1 # count has not changed
-
-  let (name, exp) = splitDefinition(def)
-  result = quote do:
-    let myAddr = addr `exp`
-    template `name`: untyped = myAddr[]
-
-macro byPtr*(def: untyped): untyped =
-  ## Defines a ptr alias for expressions. Caution: this uses `unsafeAddr`, so
-  ## is unsafe to use in general, and `byRef` should be preferred when possible.
-  ## This can for example be used on `let` variables.
-  let (name, exp) = splitDefinition(def)
-  result = quote do:
-    let myAddr = unsafeAddr `exp`
-    template `name`: untyped = myAddr[]
diff --git a/lib/pure/sugar.nim b/lib/pure/sugar.nim
index e7ef633093509..bc2f4b751ba09 100644
--- a/lib/pure/sugar.nim
+++ b/lib/pure/sugar.nim
@@ -364,3 +364,37 @@ when (NimMajor, NimMinor) >= (1, 1):
         of "bird": "word"
         else: d
     assert z == @["word", "word"]
+
+proc splitDefinition*(def: NimNode): tuple[lhs: NimNode, rhs: NimNode] =
+  ## allows library constructs such as: `byRef: a2 = expr`
+  doAssert def.kind == nnkStmtList and def.len == 1
+  let def2 = def[0]
+  doAssert def2.kind == nnkAsgn
+  let lhs = def2[0]
+  let rhs = def2[1]
+  expectKind(lhs, nnkIdent)
+  return (lhs, rhs)
+
+macro byRef*(def: untyped): untyped =
+  ## Defines a ref alias for lvalue expressions. The expression is evaluated
+  ## only once, and any side effects will only be evaluated once, at declaration
+  ## time.
+  runnableExamples:
+    var x = @[1,2,3]
+    byRef: x1=x[1]
+    x1+=10
+    doAssert type(x1) is int and x == @[1,12,3]
+
+  let (name, exp) = splitDefinition(def)
+  result = quote do:
+    let myAddr = addr `exp`
+    template `name`: untyped = myAddr[]
+
+macro byPtr*(def: untyped): untyped =
+  ## Defines a ptr alias for expressions. Caution: this uses `unsafeAddr`, so
+  ## is unsafe to use in general, and `byRef` should be preferred when possible.
+  ## This can for example be used on `let` variables.
+  let (name, exp) = splitDefinition(def)
+  result = quote do:
+    let myAddr = unsafeAddr `exp`
+    template `name`: untyped = myAddr[]
diff --git a/tests/stdlib/tsugar.nim b/tests/stdlib/tsugar.nim
new file mode 100644
index 0000000000000..3118950bd325a
--- /dev/null
+++ b/tests/stdlib/tsugar.nim
@@ -0,0 +1,63 @@
+discard """
+  output: "@[@[], @[], @[], @[], @[]]"
+"""
+import sugar
+import macros
+
+block distinctBase:
+  block:
+    type
+      Foo[T] = distinct seq[T]
+    var a: Foo[int]
+    doAssert a.type.distinctBase is seq[int]
+
+  block:
+    # simplified from https://github.com/nim-lang/Nim/pull/8531#issuecomment-410436458
+    macro uintImpl(bits: static[int]): untyped =
+      if bits >= 128:
+        let inner = getAST(uintImpl(bits div 2))
+        result = newTree(nnkBracketExpr, ident("UintImpl"), inner)
+      else:
+        result = ident("uint64")
+
+    type
+      BaseUint = UintImpl or SomeUnsignedInt
+      UintImpl[Baseuint] = object
+      Uint[bits: static[int]] = distinct uintImpl(bits)
+
+    doAssert Uint[128].distinctBase is UintImpl[uint64]
+
+block byRefBlock:
+  var count = 0
+  proc identity(a: int): auto =
+    block: count.inc; a
+  var x = @[1,2,3]
+  byRef: x1=x[identity(1)] # the lvalue expression is evaluated only here
+  doAssert count == 1
+  x1 += 10
+  doAssert type(x1) is int # use x1 just like a normal variable
+  doAssert x == @[1,12,3]
+  doAssert count == 1 # count has not changed
+  doAssert compiles (block: byRef: x2=x[0])
+  doAssert not compiles (block: byRef: x2=y[0])
+    # correctly does not compile when using invalid lvalue expression
+
+block byPtrfBlock:
+  type Foo = object
+    x: string
+  proc fun(a: Foo): auto =
+    doAssert not compiles (block: byRef: x=a.x)
+    byPtr: x=a.x
+    x[0]='X'
+  let foo = Foo(x: "asdf")
+  fun(foo)
+  doAssert foo.x == "Xsdf"
+
+# bug #7816
+import sequtils
+
+proc tester[T](x: T) =
+  let test = toSeq(0..4).map(i => newSeq[int]())
+  echo test
+
+tester(1)

From 042fd91900812179c5a9af14c9598a119057c0b4 Mon Sep 17 00:00:00 2001
From: Timothee Cour <timothee.cour2@gmail.com>
Date: Mon, 29 Jul 2019 12:20:40 -0700
Subject: [PATCH 03/14] byRef with option to export

---
 lib/pure/sugar.nim      | 34 +++++++++++++++++++++++-----------
 tests/stdlib/msugar.nim |  9 +++++++++
 tests/stdlib/tsugar.nim | 22 ++++++++++++++--------
 3 files changed, 46 insertions(+), 19 deletions(-)
 create mode 100644 tests/stdlib/msugar.nim

diff --git a/lib/pure/sugar.nim b/lib/pure/sugar.nim
index bc2f4b751ba09..ad78b7660f652 100644
--- a/lib/pure/sugar.nim
+++ b/lib/pure/sugar.nim
@@ -365,15 +365,23 @@ when (NimMajor, NimMinor) >= (1, 1):
         else: d
     assert z == @["word", "word"]
 
-proc splitDefinition*(def: NimNode): tuple[lhs: NimNode, rhs: NimNode] =
-  ## allows library constructs such as: `byRef: a2 = expr`
+proc splitDefinition*(def: NimNode): tuple[lhs: NimNode, rhs: NimNode, exported: bool] =
+  ## allows library constructs such as:
+  ## `byRef: a2=expr`
+  ## `byRef: a2*=expr` (to indicate `export`)
   doAssert def.kind == nnkStmtList and def.len == 1
   let def2 = def[0]
-  doAssert def2.kind == nnkAsgn
-  let lhs = def2[0]
-  let rhs = def2[1]
-  expectKind(lhs, nnkIdent)
-  return (lhs, rhs)
+  case def2.kind
+  of nnkInfix:
+    doAssert $def2[0].ident == "*="
+    result.lhs = def2[1]
+    result.rhs = def2[2]
+    result.exported = true
+  of nnkAsgn:
+    result.lhs = def2[0]
+    result.rhs = def2[1]
+  else: doAssert false, $def2.kind
+  expectKind(result.lhs, nnkIdent)
 
 macro byRef*(def: untyped): untyped =
   ## Defines a ref alias for lvalue expressions. The expression is evaluated
@@ -385,16 +393,20 @@ macro byRef*(def: untyped): untyped =
     x1+=10
     doAssert type(x1) is int and x == @[1,12,3]
 
-  let (name, exp) = splitDefinition(def)
+  let (name, exp, exported) = splitDefinition(def)
   result = quote do:
     let myAddr = addr `exp`
     template `name`: untyped = myAddr[]
+  if exported:
+    result.add quote do: export `name`
 
 macro byPtr*(def: untyped): untyped =
-  ## Defines a ptr alias for expressions. Caution: this uses `unsafeAddr`, so
-  ## is unsafe to use in general, and `byRef` should be preferred when possible.
+  ## Same as `byRef` but uses uses `unsafeAddr` instead of `addr`; `byRef` is
+  ## safer and should be preferred when possible.
   ## This can for example be used on `let` variables.
-  let (name, exp) = splitDefinition(def)
+  let (name, exp, exported) = splitDefinition(def)
   result = quote do:
     let myAddr = unsafeAddr `exp`
     template `name`: untyped = myAddr[]
+  if exported:
+    result.add quote do: export `name`
diff --git a/tests/stdlib/msugar.nim b/tests/stdlib/msugar.nim
new file mode 100644
index 0000000000000..f4c62cb0f0034
--- /dev/null
+++ b/tests/stdlib/msugar.nim
@@ -0,0 +1,9 @@
+import std/sugar
+
+type Bar = object
+  x: int
+type Foo = object
+  bar: Bar
+
+var foo*: Foo
+byRef: barx*=foo.bar.x
diff --git a/tests/stdlib/tsugar.nim b/tests/stdlib/tsugar.nim
index 3118950bd325a..226353a49d901 100644
--- a/tests/stdlib/tsugar.nim
+++ b/tests/stdlib/tsugar.nim
@@ -27,6 +27,16 @@ block distinctBase:
 
     doAssert Uint[128].distinctBase is UintImpl[uint64]
 
+# bug #7816
+import sequtils
+
+proc tester[T](x: T) =
+  let test = toSeq(0..4).map(i => newSeq[int]())
+  echo test
+
+tester(1)
+
+
 block byRefBlock:
   var count = 0
   proc identity(a: int): auto =
@@ -53,11 +63,7 @@ block byPtrfBlock:
   fun(foo)
   doAssert foo.x == "Xsdf"
 
-# bug #7816
-import sequtils
-
-proc tester[T](x: T) =
-  let test = toSeq(0..4).map(i => newSeq[int]())
-  echo test
-
-tester(1)
+# test byRef with export
+import ./msugar
+barx += 10
+doAssert $foo == "(bar: (x: 10))"

From cc377675e1a6e67ffae984829a5b235a6a129f92 Mon Sep 17 00:00:00 2001
From: Timothee Cour <timothee.cour2@gmail.com>
Date: Mon, 29 Jul 2019 13:27:52 -0700
Subject: [PATCH 04/14] fixup

---
 lib/pure/sugar.nim | 24 +++++++++++++++---------
 1 file changed, 15 insertions(+), 9 deletions(-)

diff --git a/lib/pure/sugar.nim b/lib/pure/sugar.nim
index ad78b7660f652..cd5215a95700d 100644
--- a/lib/pure/sugar.nim
+++ b/lib/pure/sugar.nim
@@ -373,7 +373,7 @@ proc splitDefinition*(def: NimNode): tuple[lhs: NimNode, rhs: NimNode, exported:
   let def2 = def[0]
   case def2.kind
   of nnkInfix:
-    doAssert $def2[0].ident == "*="
+    doAssert def2[0].strVal == "*="
     result.lhs = def2[1]
     result.rhs = def2[2]
     result.exported = true
@@ -394,19 +394,25 @@ macro byRef*(def: untyped): untyped =
     doAssert type(x1) is int and x == @[1,12,3]
 
   let (name, exp, exported) = splitDefinition(def)
-  result = quote do:
-    let myAddr = addr `exp`
-    template `name`: untyped = myAddr[]
   if exported:
-    result.add quote do: export `name`
+    result = quote do:
+      let myAddr = addr `exp`
+      template `name`*: untyped = myAddr[]
+  else:
+    result = quote do:
+      let myAddr = addr `exp`
+      template `name`: untyped = myAddr[]
 
 macro byPtr*(def: untyped): untyped =
   ## Same as `byRef` but uses uses `unsafeAddr` instead of `addr`; `byRef` is
   ## safer and should be preferred when possible.
   ## This can for example be used on `let` variables.
   let (name, exp, exported) = splitDefinition(def)
-  result = quote do:
-    let myAddr = unsafeAddr `exp`
-    template `name`: untyped = myAddr[]
   if exported:
-    result.add quote do: export `name`
+    result = quote do:
+      let myAddr = unsafeAddr `exp`
+      template `name`*: untyped = myAddr[]
+  else:
+    result = quote do:
+      let myAddr = unsafeAddr `exp`
+      template `name`: untyped = myAddr[]

From cda97f1532849a85fcb891a510ac5632ee43e7a4 Mon Sep 17 00:00:00 2001
From: Timothee Cour <timothee.cour2@gmail.com>
Date: Sat, 11 Jan 2020 17:53:10 -0800
Subject: [PATCH 05/14] fixup

---
 tests/stdlib/tstring.nim | 17 ---------------
 tests/stdlib/tsugar.nim  | 45 ++++++++--------------------------------
 2 files changed, 9 insertions(+), 53 deletions(-)

diff --git a/tests/stdlib/tstring.nim b/tests/stdlib/tstring.nim
index 36a2e9800e2d9..0573ee8d70888 100644
--- a/tests/stdlib/tstring.nim
+++ b/tests/stdlib/tstring.nim
@@ -1,8 +1,3 @@
-discard """
-  output: '''OK
-@[@[], @[], @[], @[], @[]]
-'''
-"""
 const characters = "abcdefghijklmnopqrstuvwxyz"
 const numbers = "1234567890"
 
@@ -79,15 +74,3 @@ proc test_string_cmp() =
 test_string_slice()
 test_string_cmp()
 
-
-#--------------------------
-# bug #7816
-import sugar
-import sequtils
-
-proc tester[T](x: T) =
-  let test = toSeq(0..4).map(i => newSeq[int]())
-  echo test
-
-tester(1)
-
diff --git a/tests/stdlib/tsugar.nim b/tests/stdlib/tsugar.nim
index 226353a49d901..7a0404801d15a 100644
--- a/tests/stdlib/tsugar.nim
+++ b/tests/stdlib/tsugar.nim
@@ -1,42 +1,6 @@
-discard """
-  output: "@[@[], @[], @[], @[], @[]]"
-"""
 import sugar
 import macros
 
-block distinctBase:
-  block:
-    type
-      Foo[T] = distinct seq[T]
-    var a: Foo[int]
-    doAssert a.type.distinctBase is seq[int]
-
-  block:
-    # simplified from https://github.com/nim-lang/Nim/pull/8531#issuecomment-410436458
-    macro uintImpl(bits: static[int]): untyped =
-      if bits >= 128:
-        let inner = getAST(uintImpl(bits div 2))
-        result = newTree(nnkBracketExpr, ident("UintImpl"), inner)
-      else:
-        result = ident("uint64")
-
-    type
-      BaseUint = UintImpl or SomeUnsignedInt
-      UintImpl[Baseuint] = object
-      Uint[bits: static[int]] = distinct uintImpl(bits)
-
-    doAssert Uint[128].distinctBase is UintImpl[uint64]
-
-# bug #7816
-import sequtils
-
-proc tester[T](x: T) =
-  let test = toSeq(0..4).map(i => newSeq[int]())
-  echo test
-
-tester(1)
-
-
 block byRefBlock:
   var count = 0
   proc identity(a: int): auto =
@@ -67,3 +31,12 @@ block byPtrfBlock:
 import ./msugar
 barx += 10
 doAssert $foo == "(bar: (x: 10))"
+
+# bug #7816
+import sequtils
+
+proc tester[T](x: T) =
+  let test = toSeq(0..4).map(i => newSeq[int]())
+  doAssert $test == """@[@[], @[], @[], @[], @[]]"""
+
+tester(1)

From a9cd64478d2aba9693cef1700ac55b511eccfb19 Mon Sep 17 00:00:00 2001
From: Timothee Cour <timothee.cour2@gmail.com>
Date: Sat, 11 Jan 2020 17:55:27 -0800
Subject: [PATCH 06/14] add changelog + since

---
 changelog.md       | 2 ++
 lib/pure/sugar.nim | 6 +++---
 2 files changed, 5 insertions(+), 3 deletions(-)

diff --git a/changelog.md b/changelog.md
index 8fac8dab6898d..126e8f55f0bd9 100644
--- a/changelog.md
+++ b/changelog.md
@@ -55,6 +55,8 @@
 
 - Added `os.isRelativeTo` to tell whether a path is relative to another
 
+- Added `sugar.byRef,byPtr` allowing a simple syntax for lvalue expressions
+
 ## Library changes
 
 - `asyncdispatch.drain` now properly takes into account `selector.hasPendingOperations`
diff --git a/lib/pure/sugar.nim b/lib/pure/sugar.nim
index cd5215a95700d..2bf7af470a746 100644
--- a/lib/pure/sugar.nim
+++ b/lib/pure/sugar.nim
@@ -365,7 +365,7 @@ when (NimMajor, NimMinor) >= (1, 1):
         else: d
     assert z == @["word", "word"]
 
-proc splitDefinition*(def: NimNode): tuple[lhs: NimNode, rhs: NimNode, exported: bool] =
+proc splitDefinition*(def: NimNode): tuple[lhs: NimNode, rhs: NimNode, exported: bool] {.since: (1,1).} =
   ## allows library constructs such as:
   ## `byRef: a2=expr`
   ## `byRef: a2*=expr` (to indicate `export`)
@@ -383,7 +383,7 @@ proc splitDefinition*(def: NimNode): tuple[lhs: NimNode, rhs: NimNode, exported:
   else: doAssert false, $def2.kind
   expectKind(result.lhs, nnkIdent)
 
-macro byRef*(def: untyped): untyped =
+macro byRef*(def: untyped): untyped {.since: (1,1).} =
   ## Defines a ref alias for lvalue expressions. The expression is evaluated
   ## only once, and any side effects will only be evaluated once, at declaration
   ## time.
@@ -403,7 +403,7 @@ macro byRef*(def: untyped): untyped =
       let myAddr = addr `exp`
       template `name`: untyped = myAddr[]
 
-macro byPtr*(def: untyped): untyped =
+macro byPtr*(def: untyped): untyped {.since: (1,1).} =
   ## Same as `byRef` but uses uses `unsafeAddr` instead of `addr`; `byRef` is
   ## safer and should be preferred when possible.
   ## This can for example be used on `let` variables.

From 87d6cf9a2fe2baf3cc7eb3142e6a112684a6780c Mon Sep 17 00:00:00 2001
From: Timothee Cour <timothee.cour2@gmail.com>
Date: Mon, 13 Jan 2020 10:33:49 -0800
Subject: [PATCH 07/14] removed byPtr

---
 changelog.md            |  2 +-
 lib/pure/sugar.nim      | 14 --------------
 tests/stdlib/tsugar.nim |  3 ---
 3 files changed, 1 insertion(+), 18 deletions(-)

diff --git a/changelog.md b/changelog.md
index 126e8f55f0bd9..e2e8c13d2e595 100644
--- a/changelog.md
+++ b/changelog.md
@@ -55,7 +55,7 @@
 
 - Added `os.isRelativeTo` to tell whether a path is relative to another
 
-- Added `sugar.byRef,byPtr` allowing a simple syntax for lvalue expressions
+- Added `sugar.byRef` allowing a ref syntax for lvalue expressions
 
 ## Library changes
 
diff --git a/lib/pure/sugar.nim b/lib/pure/sugar.nim
index 2bf7af470a746..badd2c77b68a4 100644
--- a/lib/pure/sugar.nim
+++ b/lib/pure/sugar.nim
@@ -402,17 +402,3 @@ macro byRef*(def: untyped): untyped {.since: (1,1).} =
     result = quote do:
       let myAddr = addr `exp`
       template `name`: untyped = myAddr[]
-
-macro byPtr*(def: untyped): untyped {.since: (1,1).} =
-  ## Same as `byRef` but uses uses `unsafeAddr` instead of `addr`; `byRef` is
-  ## safer and should be preferred when possible.
-  ## This can for example be used on `let` variables.
-  let (name, exp, exported) = splitDefinition(def)
-  if exported:
-    result = quote do:
-      let myAddr = unsafeAddr `exp`
-      template `name`*: untyped = myAddr[]
-  else:
-    result = quote do:
-      let myAddr = unsafeAddr `exp`
-      template `name`: untyped = myAddr[]
diff --git a/tests/stdlib/tsugar.nim b/tests/stdlib/tsugar.nim
index 7a0404801d15a..a61fbd7896fd0 100644
--- a/tests/stdlib/tsugar.nim
+++ b/tests/stdlib/tsugar.nim
@@ -21,11 +21,8 @@ block byPtrfBlock:
     x: string
   proc fun(a: Foo): auto =
     doAssert not compiles (block: byRef: x=a.x)
-    byPtr: x=a.x
-    x[0]='X'
   let foo = Foo(x: "asdf")
   fun(foo)
-  doAssert foo.x == "Xsdf"
 
 # test byRef with export
 import ./msugar

From ad8a7312ce70ceb9fb143deba5f95f36d5cfe7a0 Mon Sep 17 00:00:00 2001
From: Timothee Cour <timothee.cour2@gmail.com>
Date: Sat, 18 Jan 2020 01:58:15 -0800
Subject: [PATCH 08/14] byRef => byAddr

---
 changelog.md            |  2 +-
 lib/pure/sugar.nim      |  8 ++++----
 tests/stdlib/msugar.nim |  2 +-
 tests/stdlib/tsugar.nim | 12 ++++++------
 4 files changed, 12 insertions(+), 12 deletions(-)

diff --git a/changelog.md b/changelog.md
index e2e8c13d2e595..59a4563613461 100644
--- a/changelog.md
+++ b/changelog.md
@@ -55,7 +55,7 @@
 
 - Added `os.isRelativeTo` to tell whether a path is relative to another
 
-- Added `sugar.byRef` allowing a ref syntax for lvalue expressions
+- Added `sugar.byAddr` allowing a ref syntax for lvalue expressions
 
 ## Library changes
 
diff --git a/lib/pure/sugar.nim b/lib/pure/sugar.nim
index badd2c77b68a4..96a8e0fd22145 100644
--- a/lib/pure/sugar.nim
+++ b/lib/pure/sugar.nim
@@ -367,8 +367,8 @@ when (NimMajor, NimMinor) >= (1, 1):
 
 proc splitDefinition*(def: NimNode): tuple[lhs: NimNode, rhs: NimNode, exported: bool] {.since: (1,1).} =
   ## allows library constructs such as:
-  ## `byRef: a2=expr`
-  ## `byRef: a2*=expr` (to indicate `export`)
+  ## `byAddr: a2=expr`
+  ## `byAddr: a2*=expr` (to indicate `export`)
   doAssert def.kind == nnkStmtList and def.len == 1
   let def2 = def[0]
   case def2.kind
@@ -383,13 +383,13 @@ proc splitDefinition*(def: NimNode): tuple[lhs: NimNode, rhs: NimNode, exported:
   else: doAssert false, $def2.kind
   expectKind(result.lhs, nnkIdent)
 
-macro byRef*(def: untyped): untyped {.since: (1,1).} =
+macro byAddr*(def: untyped): untyped {.since: (1,1).} =
   ## Defines a ref alias for lvalue expressions. The expression is evaluated
   ## only once, and any side effects will only be evaluated once, at declaration
   ## time.
   runnableExamples:
     var x = @[1,2,3]
-    byRef: x1=x[1]
+    byAddr: x1=x[1]
     x1+=10
     doAssert type(x1) is int and x == @[1,12,3]
 
diff --git a/tests/stdlib/msugar.nim b/tests/stdlib/msugar.nim
index f4c62cb0f0034..fe17f1443fb76 100644
--- a/tests/stdlib/msugar.nim
+++ b/tests/stdlib/msugar.nim
@@ -6,4 +6,4 @@ type Foo = object
   bar: Bar
 
 var foo*: Foo
-byRef: barx*=foo.bar.x
+byAddr: barx*=foo.bar.x
diff --git a/tests/stdlib/tsugar.nim b/tests/stdlib/tsugar.nim
index a61fbd7896fd0..2da13489afcb0 100644
--- a/tests/stdlib/tsugar.nim
+++ b/tests/stdlib/tsugar.nim
@@ -1,30 +1,30 @@
 import sugar
 import macros
 
-block byRefBlock:
+block byAddrBlock:
   var count = 0
   proc identity(a: int): auto =
     block: count.inc; a
   var x = @[1,2,3]
-  byRef: x1=x[identity(1)] # the lvalue expression is evaluated only here
+  byAddr: x1=x[identity(1)] # the lvalue expression is evaluated only here
   doAssert count == 1
   x1 += 10
   doAssert type(x1) is int # use x1 just like a normal variable
   doAssert x == @[1,12,3]
   doAssert count == 1 # count has not changed
-  doAssert compiles (block: byRef: x2=x[0])
-  doAssert not compiles (block: byRef: x2=y[0])
+  doAssert compiles (block: byAddr: x2=x[0])
+  doAssert not compiles (block: byAddr: x2=y[0])
     # correctly does not compile when using invalid lvalue expression
 
 block byPtrfBlock:
   type Foo = object
     x: string
   proc fun(a: Foo): auto =
-    doAssert not compiles (block: byRef: x=a.x)
+    doAssert not compiles (block: byAddr: x=a.x)
   let foo = Foo(x: "asdf")
   fun(foo)
 
-# test byRef with export
+# test byAddr with export
 import ./msugar
 barx += 10
 doAssert $foo == "(bar: (x: 10))"

From 6b48b9d554ced9d5aad8479833b70500455e3840 Mon Sep 17 00:00:00 2001
From: Timothee Cour <timothee.cour2@gmail.com>
Date: Sun, 19 Jan 2020 16:55:39 -0800
Subject: [PATCH 09/14] _

---
 lib/pure/sugar.nim | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/pure/sugar.nim b/lib/pure/sugar.nim
index 96a8e0fd22145..00e91c591198d 100644
--- a/lib/pure/sugar.nim
+++ b/lib/pure/sugar.nim
@@ -369,7 +369,7 @@ proc splitDefinition*(def: NimNode): tuple[lhs: NimNode, rhs: NimNode, exported:
   ## allows library constructs such as:
   ## `byAddr: a2=expr`
   ## `byAddr: a2*=expr` (to indicate `export`)
-  doAssert def.kind == nnkStmtList and def.len == 1
+  doAssert def.kind == nnkStmtList and def.len == 1, $(def.repr, def.treeRepr)
   let def2 = def[0]
   case def2.kind
   of nnkInfix:

From 0e66b9a8bb71c9a4c9f0ea243f13b6f1c48b78d8 Mon Sep 17 00:00:00 2001
From: Timothee Cour <timothee.cour2@gmail.com>
Date: Thu, 23 Jan 2020 02:42:57 -0800
Subject: [PATCH 10/14] fix comments

---
 changelog.md       | 2 +-
 lib/pure/sugar.nim | 6 +++---
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/changelog.md b/changelog.md
index 59a4563613461..0626f36b7ea39 100644
--- a/changelog.md
+++ b/changelog.md
@@ -55,7 +55,7 @@
 
 - Added `os.isRelativeTo` to tell whether a path is relative to another
 
-- Added `sugar.byAddr` allowing a ref syntax for lvalue expressions
+- Added `sugar.byAddr` allowing a reference syntax for lvalue expressions, analog to C++ `auto& a = expr`.
 
 ## Library changes
 
diff --git a/lib/pure/sugar.nim b/lib/pure/sugar.nim
index 00e91c591198d..c6c3010486d0c 100644
--- a/lib/pure/sugar.nim
+++ b/lib/pure/sugar.nim
@@ -384,9 +384,9 @@ proc splitDefinition*(def: NimNode): tuple[lhs: NimNode, rhs: NimNode, exported:
   expectKind(result.lhs, nnkIdent)
 
 macro byAddr*(def: untyped): untyped {.since: (1,1).} =
-  ## Defines a ref alias for lvalue expressions. The expression is evaluated
-  ## only once, and any side effects will only be evaluated once, at declaration
-  ## time.
+  ## Defines a reference syntax for lvalue expressions, analog to C++ `auto& a = expr`.
+  ## The expression is evaluated only once, and any side effects will only be
+  ## evaluated once, at declaration time.
   runnableExamples:
     var x = @[1,2,3]
     byAddr: x1=x[1]

From 1d6b6bd617b8f8284ace6b8167b65d567bca5e70 Mon Sep 17 00:00:00 2001
From: Timothee Cour <timothee.cour2@gmail.com>
Date: Thu, 23 Jan 2020 02:45:36 -0800
Subject: [PATCH 11/14] address comment

---
 lib/pure/sugar.nim | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/lib/pure/sugar.nim b/lib/pure/sugar.nim
index c6c3010486d0c..c4c67c84c094e 100644
--- a/lib/pure/sugar.nim
+++ b/lib/pure/sugar.nim
@@ -369,7 +369,8 @@ proc splitDefinition*(def: NimNode): tuple[lhs: NimNode, rhs: NimNode, exported:
   ## allows library constructs such as:
   ## `byAddr: a2=expr`
   ## `byAddr: a2*=expr` (to indicate `export`)
-  doAssert def.kind == nnkStmtList and def.len == 1, $(def.repr, def.treeRepr)
+  def.expectKind nnkStmtList
+  def.expectLen 1
   let def2 = def[0]
   case def2.kind
   of nnkInfix:

From edf69603b66f3a819d2428d23bc14c5c6f10f104 Mon Sep 17 00:00:00 2001
From: Timothee Cour <timothee.cour2@gmail.com>
Date: Thu, 23 Jan 2020 03:05:39 -0800
Subject: [PATCH 12/14] disallow export: byAddr: barx*=foo.bar.x

---
 lib/pure/sugar.nim      | 12 +++++-------
 tests/stdlib/msugar.nim | 10 ++++++----
 tests/stdlib/tsugar.nim |  7 ++++---
 3 files changed, 15 insertions(+), 14 deletions(-)

diff --git a/lib/pure/sugar.nim b/lib/pure/sugar.nim
index c4c67c84c094e..bfaed5a4a695e 100644
--- a/lib/pure/sugar.nim
+++ b/lib/pure/sugar.nim
@@ -396,10 +396,8 @@ macro byAddr*(def: untyped): untyped {.since: (1,1).} =
 
   let (name, exp, exported) = splitDefinition(def)
   if exported:
-    result = quote do:
-      let myAddr = addr `exp`
-      template `name`*: untyped = myAddr[]
-  else:
-    result = quote do:
-      let myAddr = addr `exp`
-      template `name`: untyped = myAddr[]
+    error("export marker * not allowed for byAddr because of memory safety concerns", def)
+  # doAssert not exported, "export marker * not allowed for byAddr because of memory safety concerns"
+  result = quote do:
+    let myAddr = addr `exp`
+    template `name`: untyped = myAddr[]
diff --git a/tests/stdlib/msugar.nim b/tests/stdlib/msugar.nim
index fe17f1443fb76..ac5ce3ac51b8b 100644
--- a/tests/stdlib/msugar.nim
+++ b/tests/stdlib/msugar.nim
@@ -1,9 +1,11 @@
-import std/sugar
-
 type Bar = object
   x: int
 type Foo = object
   bar: Bar
 
-var foo*: Foo
-byAddr: barx*=foo.bar.x
+const exportEnabled* = false
+
+when exportEnabled:
+  import std/sugar
+  var foo*: Foo
+  byAddr: barx*=foo.bar.x
diff --git a/tests/stdlib/tsugar.nim b/tests/stdlib/tsugar.nim
index 2da13489afcb0..52689fc1aeea6 100644
--- a/tests/stdlib/tsugar.nim
+++ b/tests/stdlib/tsugar.nim
@@ -24,10 +24,11 @@ block byPtrfBlock:
   let foo = Foo(x: "asdf")
   fun(foo)
 
-# test byAddr with export
 import ./msugar
-barx += 10
-doAssert $foo == "(bar: (x: 10))"
+when exportEnabled:
+  # test byAddr with export
+  barx += 10
+  doAssert $foo == "(bar: (x: 10))"
 
 # bug #7816
 import sequtils

From 2bec2dc1b687c1e2d3a9db7fe42187448dc0977b Mon Sep 17 00:00:00 2001
From: Timothee Cour <timothee.cour2@gmail.com>
Date: Thu, 23 Jan 2020 03:09:47 -0800
Subject: [PATCH 13/14] now using explicit export in test

---
 tests/stdlib/msugar.nim | 11 +++++------
 tests/stdlib/tsugar.nim |  7 +++----
 2 files changed, 8 insertions(+), 10 deletions(-)

diff --git a/tests/stdlib/msugar.nim b/tests/stdlib/msugar.nim
index ac5ce3ac51b8b..3a2d2fc07e41f 100644
--- a/tests/stdlib/msugar.nim
+++ b/tests/stdlib/msugar.nim
@@ -1,11 +1,10 @@
+import std/sugar
+
 type Bar = object
   x: int
 type Foo = object
   bar: Bar
 
-const exportEnabled* = false
-
-when exportEnabled:
-  import std/sugar
-  var foo*: Foo
-  byAddr: barx*=foo.bar.x
+var foo*: Foo
+byAddr: barx = foo.bar.x
+export barx
diff --git a/tests/stdlib/tsugar.nim b/tests/stdlib/tsugar.nim
index 52689fc1aeea6..be0d72a470b14 100644
--- a/tests/stdlib/tsugar.nim
+++ b/tests/stdlib/tsugar.nim
@@ -25,10 +25,9 @@ block byPtrfBlock:
   fun(foo)
 
 import ./msugar
-when exportEnabled:
-  # test byAddr with export
-  barx += 10
-  doAssert $foo == "(bar: (x: 10))"
+# test byAddr with export
+barx += 10
+doAssert $foo == "(bar: (x: 10))"
 
 # bug #7816
 import sequtils

From b1e338f7b71b399ca95a62851517dcca80f6d790 Mon Sep 17 00:00:00 2001
From: Timothee Cour <timothee.cour2@gmail.com>
Date: Thu, 23 Jan 2020 03:17:42 -0800
Subject: [PATCH 14/14] improve splitDefinition

---
 lib/pure/sugar.nim | 17 ++++++++---------
 1 file changed, 8 insertions(+), 9 deletions(-)

diff --git a/lib/pure/sugar.nim b/lib/pure/sugar.nim
index bfaed5a4a695e..51d9130a04ae8 100644
--- a/lib/pure/sugar.nim
+++ b/lib/pure/sugar.nim
@@ -365,22 +365,22 @@ when (NimMajor, NimMinor) >= (1, 1):
         else: d
     assert z == @["word", "word"]
 
-proc splitDefinition*(def: NimNode): tuple[lhs: NimNode, rhs: NimNode, exported: bool] {.since: (1,1).} =
+proc splitDefinition*(def: NimNode): tuple[lhs: NimNode, rhs: NimNode, op: string] {.since: (1,1).} =
   ## allows library constructs such as:
-  ## `byAddr: a2=expr`
-  ## `byAddr: a2*=expr` (to indicate `export`)
+  ## `myDef: a2=expr`
+  ## `myDef: a2*=expr`
   def.expectKind nnkStmtList
   def.expectLen 1
   let def2 = def[0]
   case def2.kind
   of nnkInfix:
-    doAssert def2[0].strVal == "*="
     result.lhs = def2[1]
     result.rhs = def2[2]
-    result.exported = true
+    result.op = def2[0].strVal
   of nnkAsgn:
     result.lhs = def2[0]
     result.rhs = def2[1]
+    result.op = "="
   else: doAssert false, $def2.kind
   expectKind(result.lhs, nnkIdent)
 
@@ -394,10 +394,9 @@ macro byAddr*(def: untyped): untyped {.since: (1,1).} =
     x1+=10
     doAssert type(x1) is int and x == @[1,12,3]
 
-  let (name, exp, exported) = splitDefinition(def)
-  if exported:
-    error("export marker * not allowed for byAddr because of memory safety concerns", def)
-  # doAssert not exported, "export marker * not allowed for byAddr because of memory safety concerns"
+  let (name, exp, op) = splitDefinition(def)
+  if op != "=":
+    error("expected `=`, got `" & op & "`", def)
   result = quote do:
     let myAddr = addr `exp`
     template `name`: untyped = myAddr[]