improve the error message for 'attempt to redefine X'; fixes #447

4 files changed, 29 insertions, 18 deletions

diff --git a/compiler/astalgo.nim b/compiler/astalgo.nim
index 34963ee83..b716882dc 100644
--- a/compiler/astalgo.nim
+++ b/compiler/astalgo.nim
@@ -549,7 +549,8 @@ proc strTableAdd*(t: var TStrTable, n: PSym) =
   strTableRawInsert(t.data, n)
   inc(t.counter)
 
-proc strTableIncl*(t: var TStrTable, n: PSym; onConflictKeepOld=false): bool {.discardable.} =
+proc strTableInclReportConflict*(t: var TStrTable, n: PSym;
+                                 onConflictKeepOld = false): PSym =
   # returns true if n is already in the string table:
   # It is essential that `n` is written nevertheless!
   # This way the newest redefinition is picked by the semantic analyses!
@@ -564,13 +565,13 @@ proc strTableIncl*(t: var TStrTable, n: PSym; onConflictKeepOld=false): bool {.d
     # So it is possible the very same sym is added multiple
     # times to the symbol table which we allow here with the 'it == n' check.
     if it.name.id == n.name.id:
-      if it == n: return false
+      if it == n: return nil
       replaceSlot = h
     h = nextTry(h, high(t.data))
   if replaceSlot >= 0:
     if not onConflictKeepOld:
       t.data[replaceSlot] = n # overwrite it with newer definition!
-    return true             # found it
+    return t.data[replaceSlot] # found it
   elif mustRehash(len(t.data), t.counter):
     strTableEnlarge(t)
     strTableRawInsert(t.data, n)
@@ -578,7 +579,11 @@ proc strTableIncl*(t: var TStrTable, n: PSym; onConflictKeepOld=false): bool {.d
     assert(t.data[h] == nil)
     t.data[h] = n
   inc(t.counter)
-  result = false
+  result = nil
+
+proc strTableIncl*(t: var TStrTable, n: PSym;
+                   onConflictKeepOld = false): bool {.discardable.} =
+  result = strTableInclReportConflict(t, n, onConflictKeepOld) != nil
 
 proc strTableGet*(t: TStrTable, name: PIdent): PSym =
   var h: Hash = name.h and high(t.data)
diff --git a/compiler/lookups.nim b/compiler/lookups.nim
index ec9c130e3..d2e7fdcfa 100644
--- a/compiler/lookups.nim
+++ b/compiler/lookups.nim
@@ -58,8 +58,8 @@ proc considerQuotedIdent*(c: PContext; n: PNode, origin: PNode = nil): PIdent =
 template addSym*(scope: PScope, s: PSym) =
   strTableAdd(scope.symbols, s)
 
-proc addUniqueSym*(scope: PScope, s: PSym): bool =
-  result = not strTableIncl(scope.symbols, s)
+proc addUniqueSym*(scope: PScope, s: PSym): PSym =
+  result = strTableInclReportConflict(scope.symbols, s)
 
 proc openScope*(c: PContext): PScope {.discardable.} =
   result = PScope(parent: c.currentScope,
@@ -177,24 +177,30 @@ proc ensureNoMissingOrUnusedSymbols(c: PContext; scope: PScope) =
           message(c.config, s.info, hintXDeclaredButNotUsed, getSymRepr(c.config, s))
     s = nextIter(it, scope.symbols)
 
-proc wrongRedefinition*(c: PContext; info: TLineInfo, s: string) =
+proc wrongRedefinition*(c: PContext; info: TLineInfo, s: string;
+                        conflictsWith: TLineInfo) =
   if c.config.cmd != cmdInteractive:
-    localError(c.config, info, "redefinition of '$1'" % s)
+    localError(c.config, info,
+      "redefinition of '$1'; previous declaration here: $2" %
+      [s, c.config $ conflictsWith])
 
 proc addDecl*(c: PContext, sym: PSym, info: TLineInfo) =
-  if not c.currentScope.addUniqueSym(sym):
-    wrongRedefinition(c, info, sym.name.s)
+  let conflict = c.currentScope.addUniqueSym(sym)
+  if conflict != nil:
+    wrongRedefinition(c, info, sym.name.s, conflict.info)
 
 proc addDecl*(c: PContext, sym: PSym) =
-  if not c.currentScope.addUniqueSym(sym):
-    wrongRedefinition(c, sym.info, sym.name.s)
+  let conflict = c.currentScope.addUniqueSym(sym)
+  if conflict != nil:
+    wrongRedefinition(c, sym.info, sym.name.s, conflict.info)
 
 proc addPrelimDecl*(c: PContext, sym: PSym) =
   discard c.currentScope.addUniqueSym(sym)
 
 proc addDeclAt*(c: PContext; scope: PScope, sym: PSym) =
-  if not scope.addUniqueSym(sym):
-    wrongRedefinition(c, sym.info, sym.name.s)
+  let conflict = scope.addUniqueSym(sym)
+  if conflict != nil:
+    wrongRedefinition(c, sym.info, sym.name.s, conflict.info)
 
 proc addInterfaceDeclAux(c: PContext, sym: PSym) =
   if sfExported in sym.flags:
@@ -212,7 +218,7 @@ proc addOverloadableSymAt*(c: PContext; scope: PScope, fn: PSym) =
     return
   let check = strTableGet(scope.symbols, fn.name)
   if check != nil and check.kind notin OverloadableSyms:
-    wrongRedefinition(c, fn.info, fn.name.s)
+    wrongRedefinition(c, fn.info, fn.name.s, check.info)
   else:
     scope.addSym(fn)
 
diff --git a/compiler/semstmts.nim b/compiler/semstmts.nim
index 566b634af..87d144dc6 100644
--- a/compiler/semstmts.nim
+++ b/compiler/semstmts.nim
@@ -1604,7 +1604,7 @@ proc semProcAux(c: PContext, n: PNode, kind: TSymKind,
         localError(c.config, n.sons[pragmasPos].info, errPragmaOnlyInHeaderOfProcX %
           ("'" & proto.name.s & "' from " & c.config$proto.info))
     if sfForward notin proto.flags and proto.magic == mNone:
-      wrongRedefinition(c, n.info, proto.name.s)
+      wrongRedefinition(c, n.info, proto.name.s, proto.info)
     excl(proto.flags, sfForward)
     closeScope(c)         # close scope with wrong parameter symbols
     openScope(c)          # open scope for old (correct) parameter symbols
diff --git a/compiler/semtypes.nim b/compiler/semtypes.nim
index a90a06150..5394e291f 100644
--- a/compiler/semtypes.nim
+++ b/compiler/semtypes.nim
@@ -125,8 +125,8 @@ proc semEnum(c: PContext, n: PNode, prev: PType): PType =
     if sfGenSym notin e.flags:
       if not isPure: addDecl(c, e)
       else: importPureEnumField(c, e)
-    if isPure and strTableIncl(symbols, e):
-      wrongRedefinition(c, e.info, e.name.s)
+    if isPure and (let conflict = strTableInclReportConflict(symbols, e); conflict != nil):
+      wrongRedefinition(c, e.info, e.name.s, conflict.info)
     inc(counter)
   if not hasNull: incl(result.flags, tfNeedsInit)