fixes #18235 - proc annotation type macro sym leak (#18249)

* fixes #18235 - proc annotation type macro sym leak

- also fixed a typo
- proc annotations guard symbol exports with shadow scopes
- symbol handling is shadow scope aware

* test for exporting an existing unexported sym

this one is for my homie alaviss.

* Special handling not needed in semProcAnnotation

* Testcasing

* [skip ci]  clean-up and add some more comments

* [skip ci] rm trailing whitespace

Co-authored-by: Clyybber <darkmine956@gmail.com>

4 files changed, 86 insertions, 9 deletions

diff --git a/compiler/lookups.nim b/compiler/lookups.nim
index 64476d816..a21e35ddc 100644
--- a/compiler/lookups.nim
+++ b/compiler/lookups.nim
@@ -314,6 +314,7 @@ proc addDeclAt*(c: PContext; scope: PScope, sym: PSym) =
 from ic / ic import addHidden
 
 proc addInterfaceDeclAux*(c: PContext, sym: PSym, forceExport = false) =
+  ## adds symbol to the module for either private or public access.
   if sfExported in sym.flags or forceExport:
     # add to interface:
     if c.module != nil: exportSym(c, sym)
@@ -324,10 +325,15 @@ proc addInterfaceDeclAux*(c: PContext, sym: PSym, forceExport = false) =
       addHidden(c.encoder, c.packedRepr, sym)
 
 proc addInterfaceDeclAt*(c: PContext, scope: PScope, sym: PSym) =
+  ## adds a symbol on the scope and the interface if appropriate
   addDeclAt(c, scope, sym)
-  addInterfaceDeclAux(c, sym)
+  if not scope.isShadowScope:
+    # adding into a non-shadow scope, we need to handle exports, etc
+    addInterfaceDeclAux(c, sym)
 
 proc addOverloadableSymAt*(c: PContext; scope: PScope, fn: PSym) =
+  ## adds an symbol to the given scope, will check for and raise errors if it's
+  ## a redefinition as opposed to an overload.
   if fn.kind notin OverloadableSyms:
     internalError(c.config, fn.info, "addOverloadableSymAt")
     return
@@ -338,24 +344,35 @@ proc addOverloadableSymAt*(c: PContext; scope: PScope, fn: PSym) =
     scope.addSym(fn)
 
 proc addInterfaceDecl*(c: PContext, sym: PSym) =
-  # it adds the symbol to the interface if appropriate
+  ## adds a decl and the interface if appropriate
   addDecl(c, sym)
-  addInterfaceDeclAux(c, sym)
+  if not c.currentScope.isShadowScope:
+    addInterfaceDeclAux(c, sym)
 
 proc addInterfaceOverloadableSymAt*(c: PContext, scope: PScope, sym: PSym) =
-  # it adds the symbol to the interface if appropriate
+  ## adds an overloadable symbol on the scope and the interface if appropriate
   addOverloadableSymAt(c, scope, sym)
-  addInterfaceDeclAux(c, sym)
+  if not scope.isShadowScope:
+    addInterfaceDeclAux(c, sym)
 
 proc openShadowScope*(c: PContext) =
+  ## opens a shadow scope, just like any other scope except the depth is the
+  ## same as the parent -- see `isShadowScope`.
   c.currentScope = PScope(parent: c.currentScope,
                           symbols: newStrTable(),
                           depthLevel: c.scopeDepth)
 
 proc closeShadowScope*(c: PContext) =
+  ## closes the shadow scope, but doesn't merge any of the symbols
   c.closeScope
 
 proc mergeShadowScope*(c: PContext) =
+  ## close the existing scope and merge in all defined symbols, this will also
+  ## trigger any export related code if this is into a non-shadow scope.
+  ##
+  ## Merges:
+  ## shadow -> shadow: add symbols to the parent but check for redefinitions etc
+  ## shadow -> non-shadow: the above, but also handle exports and all that 
   let shadowScope = c.currentScope
   c.rawCloseScope
   for sym in shadowScope.symbols:
diff --git a/compiler/semstmts.nim b/compiler/semstmts.nim
index 8f87ce83c..d6ad7ab54 100644
--- a/compiler/semstmts.nim
+++ b/compiler/semstmts.nim
@@ -1844,7 +1844,7 @@ proc semProcAux(c: PContext, n: PNode, kind: TSymKind,
 
   # before compiling the proc params & body, set as current the scope
   # where the proc was declared
-  let delcarationScope = c.currentScope
+  let declarationScope = c.currentScope
   pushOwner(c, s)
   openScope(c)
 
@@ -1889,7 +1889,7 @@ proc semProcAux(c: PContext, n: PNode, kind: TSymKind,
 
   var (proto, comesFromShadowScope) =
       if isAnon: (nil, false)
-      else: searchForProc(c, delcarationScope, s)
+      else: searchForProc(c, declarationScope, s)
   if proto == nil and sfForward in s.flags:
     ## In cases such as a macro generating a proc with a gensymmed name we
     ## know `searchForProc` will not find it and sfForward will be set. In
@@ -1916,9 +1916,9 @@ proc semProcAux(c: PContext, n: PNode, kind: TSymKind,
 
   if not hasProto and sfGenSym notin s.flags: #and not isAnon:
     if s.kind in OverloadableSyms:
-      addInterfaceOverloadableSymAt(c, delcarationScope, s)
+      addInterfaceOverloadableSymAt(c, declarationScope, s)
     else:
-      addInterfaceDeclAt(c, delcarationScope, s)
+      addInterfaceDeclAt(c, declarationScope, s)
 
   pragmaCallable(c, s, n, validPragmas)
   if not hasProto:
diff --git a/tests/macros/m18235.nim b/tests/macros/m18235.nim
new file mode 100644
index 000000000..6bb4547e0
--- /dev/null
+++ b/tests/macros/m18235.nim
@@ -0,0 +1,42 @@
+import macros
+
+# Necessary code to update the AST on a symbol across module boundaries when
+# processed by a type macro. Used by a test of a corresponding name of this
+# file.
+
+macro eexport(n: typed): untyped =
+  result = copyNimTree(n)
+  # turn exported nnkSym -> nnkPostfix(*, nnkIdent), forcing re-sem
+  result[0] = nnkPostfix.newTree(ident"*").add:
+    n.name.strVal.ident
+
+macro unexport(n: typed): untyped =
+  result = copyNimTree(n)
+  # turn nnkSym -> nnkIdent, forcing re-sem and dropping any exported-ness
+  # that might be present
+  result[0] = n.name.strVal.ident
+
+proc foo*() {.unexport.} = discard
+proc bar() {.eexport.} = discard
+
+proc foooof*() {.unexport, eexport, unexport.} = discard
+proc barrab() {.eexport, unexport, eexport.} = discard
+
+macro eexportMulti(n: typed): untyped =
+  # use the call version of `eexport` macro for one or more decls
+  result = copyNimTree(n)
+  for i in 0..<result.len:
+    result[i] = newCall(ident"eexport", result[i])
+
+macro unexportMulti(n: typed): untyped =
+  # use the call version of `unexport` macro for one or more decls
+  result = copyNimTree(n)
+  for i in 0..<result.len:
+    result[i] = newCall(ident"unexport", result[i])
+
+unexportMulti:
+  proc oof*() = discard
+
+eexportMulti:
+  proc rab() = discard
+  proc baz*() = discard
\ No newline at end of file
diff --git a/tests/macros/t18235.nim b/tests/macros/t18235.nim
new file mode 100644
index 000000000..ba5c48a24
--- /dev/null
+++ b/tests/macros/t18235.nim
@@ -0,0 +1,18 @@
+import m18235
+
+# this must error out because it was never actually exported
+doAssert(not declared(foo))
+doAssert not compiles(foo())
+
+doAssert(not declared(foooof))
+doAssert not compiles(foooof())
+
+doAssert(not declared(oof))
+doAssert not compiles(oof())
+
+# this should have been exported just fine
+
+bar()
+barrab()
+rab()
+baz()
\ No newline at end of file