From 10ba6620e8802b51db99521fde9b49e500a44b01 Mon Sep 17 00:00:00 2001
From: metagn <10591326+metagn@users.noreply.github.com>
Date: Sun, 17 Sep 2023 13:25:51 +0300
Subject: [PATCH] whitelist symchoices

---
 compiler/condsyms.nim  |  1 -
 compiler/lineinfos.nim |  2 -
 compiler/semdata.nim   |  1 +
 compiler/semexprs.nim  | 94 ++++++++++++++++++++----------------------
 config/nim.cfg         |  4 --
 5 files changed, 45 insertions(+), 57 deletions(-)

diff --git a/compiler/condsyms.nim b/compiler/condsyms.nim
index 638cb5c1efa6..81081fd93bb1 100644
--- a/compiler/condsyms.nim
+++ b/compiler/condsyms.nim
@@ -143,7 +143,6 @@ proc initDefines*(symbols: StringTableRef) =
   defineSymbol("nimHasTemplateRedefinitionPragma")
   defineSymbol("nimHasCstringCase")
   defineSymbol("nimHasCallsitePragma")
-  defineSymbol("nimHasAmbiguousEnumHint")
 
   defineSymbol("nimHasWarnCastSizes") # deadcode
   defineSymbol("nimHasOutParams")
diff --git a/compiler/lineinfos.nim b/compiler/lineinfos.nim
index 12495aa22fd4..7cc9c68f5dd3 100644
--- a/compiler/lineinfos.nim
+++ b/compiler/lineinfos.nim
@@ -104,7 +104,6 @@ type
     hintPattern = "Pattern", hintExecuting = "Exec", hintLinking = "Link", hintDependency = "Dependency",
     hintSource = "Source", hintPerformance = "Performance", hintStackTrace = "StackTrace",
     hintGCStats = "GCStats", hintGlobalVar = "GlobalVar", hintExpandMacro = "ExpandMacro",
-    hintAmbiguousEnum = "AmbiguousEnum",
     hintUser = "User", hintUserRaw = "UserRaw", hintExtendedContext = "ExtendedContext",
     hintMsgOrigin = "MsgOrigin", # since 1.3.5
     hintDeclaredLoc = "DeclaredLoc", # since 1.5.1
@@ -225,7 +224,6 @@ const
     hintGCStats: "$1",
     hintGlobalVar: "global variable declared here",
     hintExpandMacro: "expanded macro: $1",
-    hintAmbiguousEnum: "$1",
     hintUser: "$1",
     hintUserRaw: "$1",
     hintExtendedContext: "$1",
diff --git a/compiler/semdata.nim b/compiler/semdata.nim
index 9386c3763f13..32e557f1860f 100644
--- a/compiler/semdata.nim
+++ b/compiler/semdata.nim
@@ -76,6 +76,7 @@ type
     efNoDiagnostics,
     efTypeAllowed # typeAllowed will be called after
     efWantNoDefaults
+    efAllowSymChoice # symchoice node should not be resolved
 
   TExprFlags* = set[TExprFlag]
 
diff --git a/compiler/semexprs.nim b/compiler/semexprs.nim
index d62e2561050b..0d435fb72d7e 100644
--- a/compiler/semexprs.nim
+++ b/compiler/semexprs.nim
@@ -52,7 +52,7 @@ template rejectEmptyNode(n: PNode) =
 proc semOperand(c: PContext, n: PNode, flags: TExprFlags = {}): PNode =
   rejectEmptyNode(n)
   # same as 'semExprWithType' but doesn't check for proc vars
-  result = semExpr(c, n, flags + {efOperand})
+  result = semExpr(c, n, flags + {efOperand, efAllowSymChoice})
   if result.typ != nil:
     # XXX tyGenericInst here?
     if result.typ.kind == tyProc and hasUnresolvedParams(result, {efOperand}):
@@ -90,42 +90,10 @@ proc semExprCheck(c: PContext, n: PNode, flags: TExprFlags, expectedType: PType
     # do not produce another redundant error message:
     result = errorNode(c, n)
 
-proc ambiguousSymChoice(c: PContext, orig, n: PNode): PNode =
-  let first = n[0].sym
-  var foundSym: PSym = nil
-  if first.kind == skEnumField and
-      not isAmbiguous(c, first.name, {skEnumField}, foundSym) and
-      foundSym == first:
-    # choose the first resolved enum field, i.e. the latest in scope
-    # to mirror behavior before overloadable enums
-    if hintAmbiguousEnum in c.config.notes:
-      var err = "ambiguous enum field '" & first.name.s &
-        "' assumed to be of type " & typeToString(first.typ) &
-        " -- use one of the following:\n"
-      for child in n:
-        let candidate = child.sym
-        err.add "  " & candidate.owner.name.s & "." & candidate.name.s & "\n"
-      message(c.config, orig.info, hintAmbiguousEnum, err)
-    result = n[0]
-  else:
-    var err = "ambiguous identifier '" & first.name.s &
-      "' -- use one of the following:\n"
-    for child in n:
-      let candidate = child.sym
-      err.add "  " & candidate.owner.name.s & "." & candidate.name.s
-      err.add ": " & typeToString(candidate.typ) & "\n"
-    localError(c.config, orig.info, err)
-    n.typ = errorType(c)
-    result = n
-
 proc semExprWithType(c: PContext, n: PNode, flags: TExprFlags = {}, expectedType: PType = nil): PNode =
   result = semExprCheck(c, n, flags-{efTypeAllowed}, expectedType)
   if result.typ == nil and efInTypeof in flags:
     result.typ = c.voidType
-  elif (result.typ == nil or result.typ.kind == tyNone) and
-      efTypeAllowed in flags and
-      result.kind == nkClosedSymChoice and result.len > 0:
-    result = ambiguousSymChoice(c, n, result)
   elif result.typ == nil or result.typ == c.enforceVoidContext:
     localError(c.config, n.info, errExprXHasNoType %
                 renderTree(result, {renderNoComments}))
@@ -158,6 +126,39 @@ proc semExprNoDeref(c: PContext, n: PNode, flags: TExprFlags = {}): PNode =
 proc semSymGenericInstantiation(c: PContext, n: PNode, s: PSym): PNode =
   result = symChoice(c, n, s, scClosed)
 
+proc semSym(c: PContext, n: PNode, sym: PSym, flags: TExprFlags): PNode
+
+proc isSymChoice(n: PNode): bool {.inline.} =
+  result = n.kind in nkSymChoices
+
+proc semSymChoice(c: PContext, n: PNode, flags: TExprFlags = {}, expectedType: PType = nil): PNode =
+  result = n
+  if expectedType != nil:
+    result = fitNode(c, expectedType, result, n.info)
+    if result.kind == nkSym:
+      result = semSym(c, result, result.sym, flags)
+  if isSymChoice(result) and efAllowSymChoice notin flags:
+    # some contexts might want sym choices preserved for later disambiguation
+    # in general though they are ambiguous
+    let first = n[0].sym
+    var foundSym: PSym = nil
+    if first.kind == skEnumField and
+        not isAmbiguous(c, first.name, {skEnumField}, foundSym) and
+        foundSym == first:
+      # choose the first resolved enum field, i.e. the latest in scope
+      # to mirror behavior before overloadable enums
+      result = n[0]
+    else:
+      var err = "ambiguous identifier '" & first.name.s &
+        "' -- use one of the following:\n"
+      for child in n:
+        let candidate = child.sym
+        err.add "  " & candidate.owner.name.s & "." & candidate.name.s
+        err.add ": " & typeToString(candidate.typ) & "\n"
+      localError(c.config, n.info, err)
+      n.typ = errorType(c)
+      result = n
+
 proc inlineConst(c: PContext, n: PNode, s: PSym): PNode {.inline.} =
   result = copyTree(s.astdef)
   if result.isNil:
@@ -297,9 +298,6 @@ proc isCastable(c: PContext; dst, src: PType, info: TLineInfo): bool =
   if result and src.kind == tyNil:
     return dst.size <= conf.target.ptrSize
 
-proc isSymChoice(n: PNode): bool {.inline.} =
-  result = n.kind in nkSymChoices
-
 proc maybeLiftType(t: var PType, c: PContext, info: TLineInfo) =
   # XXX: liftParamType started to perform addDecl
   # we could do that instead in semTypeNode by snooping for added
@@ -361,10 +359,10 @@ proc semConv(c: PContext, n: PNode; flags: TExprFlags = {}, expectedType: PType
   if n[1].kind == nkExprEqExpr and
       targetType.skipTypes(abstractPtrs).kind == tyObject:
     localError(c.config, n.info, "object construction uses ':', not '='")
-  var op = semExprWithType(c, n[1], flags * {efDetermineType})
-  if op.kind == nkClosedSymChoice and op.len > 0 and
-      op[0].sym.kind == skEnumField: # resolves overloadedable enums
-    op = ambiguousSymChoice(c, n, op)
+  var op = semExprWithType(c, n[1], flags * {efDetermineType} + {efAllowSymChoice})
+  if isSymChoice(op) and op[0].sym.kind notin routineKinds:
+    # T(foo) disambiguation syntax only allowed for routines
+    op = semSymChoice(c, op)
   if targetType.kind != tyGenericParam and targetType.isMetaType:
     let final = inferWithMetatype(c, targetType, op, true)
     result.add final
@@ -1057,7 +1055,7 @@ proc semIndirectOp(c: PContext, n: PNode, flags: TExprFlags; expectedType: PType
     else:
       n[0] = n0
   else:
-    n[0] = semExpr(c, n[0], {efInCall})
+    n[0] = semExpr(c, n[0], {efInCall, efAllowSymChoice})
     let t = n[0].typ
     if t != nil and t.kind in {tyVar, tyLent}:
       n[0] = newDeref(n[0])
@@ -1619,7 +1617,7 @@ proc semSubscript(c: PContext, n: PNode, flags: TExprFlags): PNode =
     return
   checkMinSonsLen(n, 2, c.config)
   # signal that generic parameters may be applied after
-  n[0] = semExprWithType(c, n[0], {efNoEvaluateGeneric})
+  n[0] = semExprWithType(c, n[0], {efNoEvaluateGeneric, efAllowSymChoice})
   var arr = skipTypes(n[0].typ, {tyGenericInst, tyUserTypeClassInst, tyOwned,
                                       tyVar, tyLent, tyPtr, tyRef, tyAlias, tySink})
   if arr.kind == tyStatic:
@@ -3059,14 +3057,14 @@ proc semExpr(c: PContext, n: PNode, flags: TExprFlags = {}, expectedType: PType
       result = enumFieldSymChoice(c, n, s)
     else:
       result = semSym(c, n, s, flags)
-    if expectedType != nil and isSymChoice(result):
-      result = fitNode(c, expectedType, result, n.info)
-      if result.kind == nkSym:
-        result = semSym(c, result, result.sym, flags)
+    if isSymChoice(result):
+      result = semSymChoice(c, result, flags, expectedType)
   of nkSym:
     # because of the changed symbol binding, this does not mean that we
     # don't have to check the symbol for semantics here again!
     result = semSym(c, n, n.sym, flags)
+  of nkClosedSymChoice, nkOpenSymChoice:
+    result = semSymChoice(c, result, flags, expectedType)
   of nkEmpty, nkNone, nkCommentStmt, nkType:
     discard
   of nkNilLit:
@@ -3263,10 +3261,6 @@ proc semExpr(c: PContext, n: PNode, flags: TExprFlags = {}, expectedType: PType
     considerGenSyms(c, n)
   of nkTableConstr:
     result = semTableConstr(c, n, expectedType)
-  of nkClosedSymChoice, nkOpenSymChoice:
-    # handling of sym choices is context dependent
-    # the node is left intact for now
-    discard
   of nkStaticExpr: result = semStaticExpr(c, n[0], expectedType)
   of nkAsgn, nkFastAsgn: result = semAsgn(c, n)
   of nkBlockStmt, nkBlockExpr: result = semBlock(c, n, flags, expectedType)
diff --git a/config/nim.cfg b/config/nim.cfg
index 7a2d5c76efe5..7c99581396df 100644
--- a/config/nim.cfg
+++ b/config/nim.cfg
@@ -18,10 +18,6 @@ cc = gcc
   hint[LineTooLong]=off
 @end
 
-@if nimHasAmbiguousEnumHint:
-  # not needed if hint is a style check
-  hint[AmbiguousEnum]=off
-@end
 #hint[XDeclaredButNotUsed]=off
 
 threads:on