compiler: better error messages (#5613)

8 files changed, 37 insertions, 29 deletions

diff --git a/compiler/lookups.nim b/compiler/lookups.nim
index 089e69ff9..5c326d10a 100644
--- a/compiler/lookups.nim
+++ b/compiler/lookups.nim
@@ -15,17 +15,28 @@ import
 
 proc ensureNoMissingOrUnusedSymbols(scope: PScope)
 
-proc considerQuotedIdent*(n: PNode): PIdent =
+proc noidentError(n, origin: PNode) =
+  var m = ""
+  if origin != nil:
+    m.add "in expression '" & origin.renderTree & "': "
+  m.add "identifier expected, but found '" & n.renderTree & "'"
+  localError(n.info, m)
+
+proc considerQuotedIdent*(n: PNode, origin: PNode = nil): PIdent =
   ## Retrieve a PIdent from a PNode, taking into account accent nodes.
+  ## ``origin`` can be nil. If it is not nil, it is used for a better
+  ## error message.
+  template handleError(n, origin: PNode) =
+    noidentError(n, origin)
+    result = getIdent"<Error>"
+
   case n.kind
   of nkIdent: result = n.ident
   of nkSym: result = n.sym.name
   of nkAccQuoted:
     case n.len
-    of 0:
-      localError(n.info, errIdentifierExpected, renderTree(n))
-      result = getIdent"<Error>"
-    of 1: result = considerQuotedIdent(n.sons[0])
+    of 0: handleError(n, origin)
+    of 1: result = considerQuotedIdent(n.sons[0], origin)
     else:
       var id = ""
       for i in 0.. <n.len:
@@ -33,14 +44,11 @@ proc considerQuotedIdent*(n: PNode): PIdent =
         case x.kind
         of nkIdent: id.add(x.ident.s)
         of nkSym: id.add(x.sym.name.s)
-        else:
-          localError(n.info, errIdentifierExpected, renderTree(n))
-          return getIdent"<Error>"
+        else: handleError(n, origin)
       result = getIdent(id)
   of nkOpenSymChoice, nkClosedSymChoice: result = n.sons[0].sym.name
   else:
-    localError(n.info, errIdentifierExpected, renderTree(n))
-    result = getIdent"<Error>"
+    handleError(n, origin)
 
 template addSym*(scope: PScope, s: PSym) =
   strTableAdd(scope.symbols, s)
@@ -353,7 +361,7 @@ proc initOverloadIter*(o: var TOverloadIter, c: PContext, n: PNode): PSym =
       if n.sons[1].kind == nkIdent:
         ident = n.sons[1].ident
       elif n.sons[1].kind == nkAccQuoted:
-        ident = considerQuotedIdent(n.sons[1])
+        ident = considerQuotedIdent(n.sons[1], n)
       if ident != nil:
         if o.m == c.module:
           # a module may access its private members:
@@ -363,8 +371,7 @@ proc initOverloadIter*(o: var TOverloadIter, c: PContext, n: PNode): PSym =
         else:
           result = initIdentIter(o.it, o.m.tab, ident).skipAlias(n)
       else:
-        localError(n.sons[1].info, errIdentifierExpected,
-                   renderTree(n.sons[1]))
+        noidentError(n.sons[1], n)
         result = errorSym(c, n.sons[1])
   of nkClosedSymChoice, nkOpenSymChoice:
     o.mode = oimSymChoice
diff --git a/compiler/semexprs.nim b/compiler/semexprs.nim
index 4d698dbfc..80b844053 100644
--- a/compiler/semexprs.nim
+++ b/compiler/semexprs.nim
@@ -355,7 +355,7 @@ proc semOpAux(c: PContext, n: PNode) =
     var a = n.sons[i]
     if a.kind == nkExprEqExpr and sonsLen(a) == 2:
       var info = a.sons[0].info
-      a.sons[0] = newIdentNode(considerQuotedIdent(a.sons[0]), info)
+      a.sons[0] = newIdentNode(considerQuotedIdent(a.sons[0], a), info)
       a.sons[1] = semExprWithType(c, a.sons[1], flags)
       a.typ = a.sons[1].typ
     else:
@@ -1076,7 +1076,7 @@ proc builtinFieldAccess(c: PContext, n: PNode, flags: TExprFlags): PNode =
 
   n.sons[0] = semExprWithType(c, n.sons[0], flags+{efDetermineType})
   #restoreOldStyleType(n.sons[0])
-  var i = considerQuotedIdent(n.sons[1])
+  var i = considerQuotedIdent(n.sons[1], n)
   var ty = n.sons[0].typ
   var f: PSym = nil
   result = nil
@@ -1160,7 +1160,7 @@ proc dotTransformation(c: PContext, n: PNode): PNode =
     addSon(result, n.sons[1])
     addSon(result, copyTree(n[0]))
   else:
-    var i = considerQuotedIdent(n.sons[1])
+    var i = considerQuotedIdent(n.sons[1], n)
     result = newNodeI(nkDotCall, n.info)
     result.flags.incl nfDotField
     addSon(result, newIdentNode(i, n[1].info))
@@ -1280,7 +1280,7 @@ proc semArrayAccess(c: PContext, n: PNode, flags: TExprFlags): PNode =
   c.p.bracketExpr = oldBracketExpr
 
 proc propertyWriteAccess(c: PContext, n, nOrig, a: PNode): PNode =
-  var id = considerQuotedIdent(a[1])
+  var id = considerQuotedIdent(a[1], a)
   var setterId = newIdentNode(getIdent(id.s & '='), n.info)
   # a[0] is already checked for semantics, that does ``builtinFieldAccess``
   # this is ugly. XXX Semantic checking should use the ``nfSem`` flag for
@@ -1529,7 +1529,7 @@ proc lookUpForDefined(c: PContext, n: PNode, onlyCurrentScope: bool): PSym =
     checkSonsLen(n, 2)
     var m = lookUpForDefined(c, n.sons[0], onlyCurrentScope)
     if m != nil and m.kind == skModule:
-      let ident = considerQuotedIdent(n[1])
+      let ident = considerQuotedIdent(n[1], n)
       if m == c.module:
         result = strTableGet(c.topLevelScope.symbols, ident)
       else:
@@ -1548,7 +1548,7 @@ proc semDefined(c: PContext, n: PNode, onlyCurrentScope: bool): PNode =
   checkSonsLen(n, 2)
   # we replace this node by a 'true' or 'false' node:
   result = newIntNode(nkIntLit, 0)
-  if not onlyCurrentScope and considerQuotedIdent(n[0]).s == "defined":
+  if not onlyCurrentScope and considerQuotedIdent(n[0], n).s == "defined":
     if n.sons[1].kind != nkIdent:
       localError(n.info, "obsolete usage of 'defined', use 'declared' instead")
     elif condsyms.isDefined(n.sons[1].ident):
@@ -2097,7 +2097,7 @@ proc semObjConstr(c: PContext, n: PNode, flags: TExprFlags): PNode =
     if it.kind != nkExprColonExpr:
       localError(n.info, errNamedExprExpected)
       break
-    let id = considerQuotedIdent(it.sons[0])
+    let id = considerQuotedIdent(it.sons[0], it)
 
     if containsOrIncl(ids, id.id):
       localError(it.info, errFieldInitTwice, id.s)
diff --git a/compiler/semstmts.nim b/compiler/semstmts.nim
index 9a1850932..f4efcc0ff 100644
--- a/compiler/semstmts.nim
+++ b/compiler/semstmts.nim
@@ -160,10 +160,11 @@ proc discardCheck(c: PContext, result: PNode) =
       else:
         var n = result
         while n.kind in skipForDiscardable: n = n.lastSon
+        var s = "expression '" & $n & "' is of type '" &
+            result.typ.typeToString & "' and has to be discarded"
         if result.typ.kind == tyProc:
-          localError(n.info, "value of type '" & result.typ.typeToString & "' has to be discarded; for a function call use ()")
-        else:
-          localError(n.info, errDiscardValueX, result.typ.typeToString)
+          s.add "; for a function call use ()"
+        localError(n.info,  s)
 
 proc semIf(c: PContext, n: PNode): PNode =
   result = n
diff --git a/tests/discard/tneedsdiscard.nim b/tests/discard/tneedsdiscard.nim
index 2df3531e7..509e8233b 100644
--- a/tests/discard/tneedsdiscard.nim
+++ b/tests/discard/tneedsdiscard.nim
@@ -1,10 +1,10 @@
 discard """
   line: 10
-  errormsg: "value of type 'bool' has to be discarded"
+  errormsg: '''expression 'open(f, "arg.txt", fmRead, -1)' is of type 'bool' and has to be discarded'''
 """
 
 proc p =
-  var f: TFile
+  var f: File
   echo "hi"
 
   open(f, "arg.txt")
diff --git a/tests/discard/tvoidcontext.nim b/tests/discard/tvoidcontext.nim
index c3ea68bae..25e0d4fdf 100644
--- a/tests/discard/tvoidcontext.nim
+++ b/tests/discard/tvoidcontext.nim
@@ -1,5 +1,5 @@
 discard """
-  errormsg: "value of type 'string' has to be discarded"
+  errormsg: '''expression '"invalid"' is of type 'string' and has to be discarded'''
   line: 12
 """
 
diff --git a/tests/errmsgs/tdont_show_system.nim b/tests/errmsgs/tdont_show_system.nim
index 6963a8a3f..830113218 100644
--- a/tests/errmsgs/tdont_show_system.nim
+++ b/tests/errmsgs/tdont_show_system.nim
@@ -1,5 +1,5 @@
 discard """
-  errormsg: "value of type 'bool' has to be discarded"
+  errormsg: "expression 'true' is of type 'bool' and has to be discarded"
   line: 13
   file: "tdont_show_system.nim"
 """
diff --git a/tests/exprs/texprstmt.nim b/tests/exprs/texprstmt.nim
index 79323d82a..0e92702e8 100644
--- a/tests/exprs/texprstmt.nim
+++ b/tests/exprs/texprstmt.nim
@@ -1,6 +1,6 @@
 discard """
   line: 10
-  errormsg: "value of type 'string' has to be discarded"
+  errormsg: "expression 'result[1 .. -(len(result), 1)]' is of type 'string' and has to be discarded"
 """
 
 # bug #578
diff --git a/tests/exprs/tstmtexp.nim b/tests/exprs/tstmtexp.nim
index fe60dd3ba..0fb835bc6 100644
--- a/tests/exprs/tstmtexp.nim
+++ b/tests/exprs/tstmtexp.nim
@@ -1,7 +1,7 @@
 discard """
   file: "tstmtexp.nim"
   line: 8
-  errormsg: "value of type 'int literal(5)' has to be discarded"
+  errormsg: "expression '5' is of type 'int literal(5)' and has to be discarded"
 """
 # Test 3