From 49d63b1528c04cf6598be9ced971a3df46fec73f Mon Sep 17 00:00:00 2001
From: zerbina <100542850+zerbina@users.noreply.github.com>
Date: Mon, 2 Sep 2024 21:05:27 +0000
Subject: [PATCH 1/2] fix(sem): crash with procedure with error used as
 argument

Summary
=======

Fix a compiler crash when passing the name of a procedure with an error
as an argument to another routine.

Details
=======

Overloadable and overloaded procedure symbols forewent the `nkError`
wrapping done by `semSym`, thus not preventing instantiation or later
inspection by `sempass2`, crashing the compiler with an `IndexDefect`
on attempting to access the symbol's AST.

Non-overloaded procedure symbols can be passed to `semSym` directly,
which takes care of the wrapping. To handle overloaded symbols,
`sigmatch.paramTypesMatch` passes the picked symbol to `semExpr` first,
which subsequently calls `semSym`, also marking the symbols as used.

The problem with using `semExpr` in `paramTypesMatch` is that the
returned `nkError` currently dismisses the overload, resulting in an
unnecessary "type mismatch" compiler error (with `nimsuggest`, `check`,
etc.).
---
 compiler/sem/semexprs.nim                      |  1 +
 compiler/sem/sigmatch.nim                      |  4 ++--
 .../tgeneric_proc_sym_as_argument.nim          | 15 +++++++++++++++
 .../toverloaded_proc_sym_as_argument.nim       | 18 ++++++++++++++++++
 .../tproc_sym_as_argument.nim                  | 15 +++++++++++++++
 5 files changed, 51 insertions(+), 2 deletions(-)
 create mode 100644 tests/error_propagation/tgeneric_proc_sym_as_argument.nim
 create mode 100644 tests/error_propagation/toverloaded_proc_sym_as_argument.nim
 create mode 100644 tests/error_propagation/tproc_sym_as_argument.nim

diff --git a/compiler/sem/semexprs.nim b/compiler/sem/semexprs.nim
index f86df14858c..13c8b139b55 100644
--- a/compiler/sem/semexprs.nim
+++ b/compiler/sem/semexprs.nim
@@ -3561,6 +3561,7 @@ proc semExpr(c: PContext, n: PNode, flags: TExprFlags = {}): PNode =
       result = symChoice(c, n, s, scClosed)
       if result.kind == nkSym:
         markIndirect(c, result.sym)
+        result = semSym(c, n, result.sym, flags)
     of skEnumField:
       if overloadableEnums in c.features:
         result = enumFieldSymChoice(c, n, s)
diff --git a/compiler/sem/sigmatch.nim b/compiler/sem/sigmatch.nim
index 73cb0c5bcaf..b155ed49551 100644
--- a/compiler/sem/sigmatch.nim
+++ b/compiler/sem/sigmatch.nim
@@ -2602,9 +2602,9 @@ proc paramTypesMatch*(
 
     else:
       # only one valid interpretation found, executing argument match
-      markUsed(candidate.c, arg.info, arg[bestArg].sym)
+      let realArg = candidate.c.semExpr(c, arg[bestArg], {})
       result = paramTypesMatchAux(
-        candidate, formal, arg[bestArg].typ, arg[bestArg])
+        candidate, formal, realArg.typ, realArg)
 
   when false:
     if candidate.calleeSym != nil and
diff --git a/tests/error_propagation/tgeneric_proc_sym_as_argument.nim b/tests/error_propagation/tgeneric_proc_sym_as_argument.nim
new file mode 100644
index 00000000000..88b600dfc60
--- /dev/null
+++ b/tests/error_propagation/tgeneric_proc_sym_as_argument.nim
@@ -0,0 +1,15 @@
+discard """
+  description: '''
+    Ensure that generic procedure symbols are wrapped in an error, when used
+    in an argument context
+  '''
+  action: reject
+  matrix: "--errorMax:100"
+"""
+
+proc call(p: proc(x: int)) = discard
+
+proc withError[T](x: T) =
+  missing
+
+call(withError) # <- this crashed the compiler
diff --git a/tests/error_propagation/toverloaded_proc_sym_as_argument.nim b/tests/error_propagation/toverloaded_proc_sym_as_argument.nim
new file mode 100644
index 00000000000..cbd2811b6d2
--- /dev/null
+++ b/tests/error_propagation/toverloaded_proc_sym_as_argument.nim
@@ -0,0 +1,18 @@
+discard """
+  description: '''
+    Ensure that overloaded procedure symbols are wrapped in an error, when
+    used in an argument context
+  '''
+  action: reject
+  matrix: "--errorMax:100"
+"""
+
+proc call(p: proc()) = discard
+
+proc withError() =
+  missing
+
+proc withError(a: int) = # <- this overload is not picked below
+  discard
+
+call(withError) # <- this crashed the compiler
diff --git a/tests/error_propagation/tproc_sym_as_argument.nim b/tests/error_propagation/tproc_sym_as_argument.nim
new file mode 100644
index 00000000000..2bc1e0f79f6
--- /dev/null
+++ b/tests/error_propagation/tproc_sym_as_argument.nim
@@ -0,0 +1,15 @@
+discard """
+  description: '''
+    Ensure that procedure symbols are wrapped in an error, when used in an
+    argument context
+  '''
+  action: reject
+  matrix: "--errorMax:100"
+"""
+
+proc call(p: proc()) = discard
+
+proc withError() =
+  missing
+
+call(withError) # <- this crashed the compiler

From d0155c032f67097cb0f0ac08057ef8090db3b1b7 Mon Sep 17 00:00:00 2001
From: zerbina <100542850+zerbina@users.noreply.github.com>
Date: Tue, 3 Sep 2024 19:14:17 +0000
Subject: [PATCH 2/2] sem: don't use `semSym` for proc-like symbols

`semSym` would call `markUsed` on the symbol again, resulting in a
duplicate usage being reported with `nimsuggest`.
---
 compiler/sem/semexprs.nim | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/compiler/sem/semexprs.nim b/compiler/sem/semexprs.nim
index 13c8b139b55..1e546b6089f 100644
--- a/compiler/sem/semexprs.nim
+++ b/compiler/sem/semexprs.nim
@@ -3561,7 +3561,10 @@ proc semExpr(c: PContext, n: PNode, flags: TExprFlags = {}): PNode =
       result = symChoice(c, n, s, scClosed)
       if result.kind == nkSym:
         markIndirect(c, result.sym)
-        result = semSym(c, n, result.sym, flags)
+        # the symbol was alrady marked as used, don't mark it as such again
+        # (via ``semSym``)
+        if result.sym.ast.isError:
+          result = c.config.newError(result, PAstDiag(kind: adWrappedSymError))
     of skEnumField:
       if overloadableEnums in c.features:
         result = enumFieldSymChoice(c, n, s)