even lighter version of #17938: fix most issues with UnusedImport, XDeclaredButNotUsed, etc; fix #17511, #17510, #14246 (without realModule) (#18362)

* {.used: symbol}

* add tests

* fix tests with --import

* --import works without giving spurious unused warnings

* new warning warnDuplicateModuleImport for `import foo; import foo`

* fix test, add resolveModuleAlias, use proper line info for module aliases

* fix spurious warnings

* fix deprecation msg for deprecated modules even with `import foo as bar`

* disable a test for i386 pending sorting XDeclaredButNotUsed errors

* UnusedImport now works with re-exported symbols

* fix typo [skip ci]

* ic support

* add genPNode to allow writing PNode-based compiler code similarly to `genAst`

* fix DuplicateModuleImport warning

* adjust test

* fixup

* fixup

* fixup

* fix after rebase

* fix for IC

* keep the proc inline, move the const out

* [skip ci] fix changelog

* experiment: remove calls to resolveModuleAlias

* followup

* fixup

* fix tests/modules/tselfimport.nim

* workaround tests/deprecated/tmodule1.nim

* fix properly

* simplify

compiler/importer.nim

@@ -13,6 +13,7 @@ import
   intsets, ast, astalgo, msgs, options, idents, lookups,
   semdata, modulepaths, sigmatch, lineinfos, sets,
   modulegraphs, wordrecg
+from strutils import `%`
 
 proc readExceptSet*(c: PContext, n: PNode): IntSet =
   assert n.kind in {nkImportExceptStmt, nkExportExceptStmt}
@@ -224,19 +225,20 @@ proc importForwarded(c: PContext, n: PNode, exceptSet: IntSet; fromMod: PSym; im
 
 proc importModuleAs(c: PContext; n: PNode, realModule: PSym, importHidden: bool): PSym =
   result = realModule
-  c.unusedImports.add((realModule, n.info))
   template createModuleAliasImpl(ident): untyped =
-    createModuleAlias(realModule, nextSymId c.idgen, ident, realModule.info, c.config.options)
+    createModuleAlias(realModule, nextSymId c.idgen, ident, n.info, c.config.options)
   if n.kind != nkImportAs: discard
   elif n.len != 2 or n[1].kind != nkIdent:
     localError(c.config, n.info, "module alias must be an identifier")
   elif n[1].ident.id != realModule.name.id:
     # some misguided guy will write 'import abc.foo as foo' ...
     result = createModuleAliasImpl(n[1].ident)
-  if importHidden:
+  if result == realModule:
-    if result == realModule: # avoids modifying `realModule`, see D20201209T194412.
+    # avoids modifying `realModule`, see D20201209T194412 for `import {.all.}`
     result = createModuleAliasImpl(realModule.name)
+  if importHidden:
     result.options.incl optImportHidden
+  c.unusedImports.add((result, n.info))
 
 proc transformImportAs(c: PContext; n: PNode): tuple[node: PNode, importHidden: bool] =
   var ret: typeof(result)
@@ -274,23 +276,22 @@ proc myImportModule(c: PContext, n: var PNode, importStmtResult: PNode): PSym =
                 toFullPath(c.config, c.graph.importStack[i+1])
       c.recursiveDep = err
 
+    var realModule: PSym
     discard pushOptionEntry(c)
-    result = importModuleAs(c, n, c.graph.importModuleCallback(c.graph, c.module, f), transf.importHidden)
+    realModule = c.graph.importModuleCallback(c.graph, c.module, f)
+    result = importModuleAs(c, n, realModule, transf.importHidden)
     popOptionEntry(c)
 
     #echo "set back to ", L
     c.graph.importStack.setLen(L)
     # we cannot perform this check reliably because of
-    # test: modules/import_in_config)
+    # test: modules/import_in_config) # xxx is that still true?
-    when true:
+    if realModule == c.module:
-      if result.info.fileIndex == c.module.info.fileIndex and
+      localError(c.config, n.info, "module '$1' cannot import itself" % realModule.name.s)
-          result.info.fileIndex == n.info.fileIndex:
+    if sfDeprecated in realModule.flags:
-        localError(c.config, n.info, "A module cannot import itself")
+      var prefix = ""
-    if sfDeprecated in result.flags:
+      if realModule.constraint != nil: prefix = realModule.constraint.strVal & "; "
-      if result.constraint != nil:
+      message(c.config, n.info, warnDeprecated, prefix & realModule.name.s & " is deprecated")
-        message(c.config, n.info, warnDeprecated, result.constraint.strVal & "; " & result.name.s & " is deprecated")
-      else:
-        message(c.config, n.info, warnDeprecated, result.name.s & " is deprecated")
     suggestSym(c.graph, n.info, result, c.graph.usageSym, false)
     importStmtResult.add newSymNode(result, n.info)
     #newStrNode(toFullPath(c.config, f), n.info)
@@ -303,6 +304,9 @@ proc impMod(c: PContext; it: PNode; importStmtResult: PNode) =
     addDecl(c, m, it.info) # add symbol to symbol table of module
     importAllSymbols(c, m)
     #importForwarded(c, m.ast, emptySet, m)
+    for s in allSyms(c.graph, m): # fixes bug #17510, for re-exported symbols
+      if s.owner != m:
+        c.exportIndirections.incl((m.id, s.id))
 
 proc evalImport*(c: PContext, n: PNode): PNode =
   result = newNodeI(nkImportStmt, n.info)

compiler/lineinfos.nim

@@ -67,6 +67,7 @@ type
     warnResultUsed = "ResultUsed",
     warnCannotOpen = "CannotOpen",
     warnFileChanged = "FileChanged",
+    warnDuplicateModuleImport = "DuplicateModuleImport",
     warnUser = "User",
     # hints
     hintSuccess = "Success", hintSuccessX = "SuccessX",
@@ -148,6 +149,7 @@ const
     warnResultUsed: "used 'result' variable",
     warnCannotOpen: "cannot open: $1",
     warnFileChanged: "file changed: $1",
+    warnDuplicateModuleImport: "$1",
     warnUser: "$1",
     hintSuccess: "operation successful: $#",
     # keep in sync with `testament.isSuccess`

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): PSym =
+proc addUniqueSym*(scope: PScope, s: PSym, onConflictKeepOld: bool): PSym =
-  result = strTableInclReportConflict(scope.symbols, s)
+  result = strTableInclReportConflict(scope.symbols, s, onConflictKeepOld)
 
 proc openScope*(c: PContext): PScope {.discardable.} =
   result = PScope(parent: c.currentScope,
@@ -288,19 +288,23 @@ proc ensureNoMissingOrUnusedSymbols(c: PContext; scope: PScope) =
     message(c.config, s.info, hintXDeclaredButNotUsed, s.name.s)
 
 proc wrongRedefinition*(c: PContext; info: TLineInfo, s: string;
-                        conflictsWith: TLineInfo) =
+                        conflictsWith: TLineInfo, note = errGenerated) =
   ## Emit a redefinition error if in non-interactive mode
   if c.config.cmd != cmdInteractive:
-    localError(c.config, info,
+    localError(c.config, info, note,
       "redefinition of '$1'; previous declaration here: $2" %
       [s, c.config $ conflictsWith])
 
 # xxx pending bootstrap >= 1.4, replace all those overloads with a single one:
 # proc addDecl*(c: PContext, sym: PSym, info = sym.info, scope = c.currentScope) {.inline.} =
 proc addDeclAt*(c: PContext; scope: PScope, sym: PSym, info: TLineInfo) =
-  let conflict = scope.addUniqueSym(sym)
+  let conflict = scope.addUniqueSym(sym, onConflictKeepOld = true)
   if conflict != nil:
-    wrongRedefinition(c, info, sym.name.s, conflict.info)
+    var note = errGenerated
+    if sym.kind == skModule and conflict.kind == skModule and sym.owner == conflict.owner:
+      # import foo; import foo
+      note = warnDuplicateModuleImport
+    wrongRedefinition(c, info, sym.name.s, conflict.info, note)
 
 proc addDeclAt*(c: PContext; scope: PScope, sym: PSym) {.inline.} =
   addDeclAt(c, scope, sym, sym.info)
@@ -312,7 +316,7 @@ proc addDecl*(c: PContext, sym: PSym) {.inline.} =
   addDeclAt(c, c.currentScope, sym)
 
 proc addPrelimDecl*(c: PContext, sym: PSym) =
-  discard c.currentScope.addUniqueSym(sym)
+  discard c.currentScope.addUniqueSym(sym, onConflictKeepOld = false)
 
 from ic / ic import addHidden
 

compiler/passaux.nim

@@ -10,7 +10,7 @@
 ## implements some little helper passes
 
 import
-  ast, passes, idents, msgs, options, lineinfos
+  ast, passes, msgs, options, lineinfos
 
 from modulegraphs import ModuleGraph, PPassContext
 

lib/system/schubfach.nim

@@ -95,14 +95,6 @@ proc floorLog2Pow10(e: int32): int32 {.inline.} =
   sf_Assert(e <= 1233)
   return floorDivPow2(e * 1741647, 19)
 
-proc computePow10Single(k: int32): uint64 {.inline.} =
-  ##  There are unique beta and r such that 10^k = beta 2^r and
-  ##  2^63 <= beta < 2^64, namely r = floor(log_2 10^k) - 63 and
-  ##  beta = 2^-r 10^k.
-  ##  Let g = ceil(beta), so (g-1) 2^r < 10^k <= g 2^r, with the latter
-  ##  value being a pretty good overestimate for 10^k.
-  ##  NB: Since for all the required exponents k, we have g < 2^64,
-  ##      all constants can be stored in 128-bit integers.
 const
   kMin: int32 = -31
   kMax: int32 = 45
@@ -132,6 +124,15 @@ proc computePow10Single(k: int32): uint64 {.inline.} =
     0xF0BDC21ABB48DB21'u64, 0x96769950B50D88F5'u64, 0xBC143FA4E250EB32'u64,
     0xEB194F8E1AE525FE'u64, 0x92EFD1B8D0CF37BF'u64, 0xB7ABC627050305AE'u64,
     0xE596B7B0C643C71A'u64, 0x8F7E32CE7BEA5C70'u64, 0xB35DBF821AE4F38C'u64]
+
+proc computePow10Single(k: int32): uint64 {.inline.} =
+  ##  There are unique beta and r such that 10^k = beta 2^r and
+  ##  2^63 <= beta < 2^64, namely r = floor(log_2 10^k) - 63 and
+  ##  beta = 2^-r 10^k.
+  ##  Let g = ceil(beta), so (g-1) 2^r < 10^k <= g 2^r, with the latter
+  ##  value being a pretty good overestimate for 10^k.
+  ##  NB: Since for all the required exponents k, we have g < 2^64,
+  ##      all constants can be stored in 128-bit integers.
   sf_Assert(k >= kMin)
   sf_Assert(k <= kMax)
   return g[k - kMin]

testament/categories.nim

@@ -508,6 +508,7 @@ proc icTests(r: var TResults; testsDir: string, cat: Category, options: string;
 
   const tempExt = "_temp.nim"
   for it in walkDirRec(testsDir):
+  # for it in ["tests/ic/timports.nim"]: # debugging: to try a specific test
     if isTestFile(it) and not it.endsWith(tempExt):
       let nimcache = nimcacheDir(it, options, getTestSpecTarget())
       removeDir(nimcache)

tests/deprecated/tmodule1.nim

@@ -1,13 +1,23 @@
 discard """
-  nimout: '''tmodule1.nim(11, 8) Warning: goodbye; importme is deprecated [Deprecated]
+  matrix: "--hint:all:off"
-tmodule1.nim(14, 10) Warning: Ty is deprecated [Deprecated]
+  nimoutFull: true
-tmodule1.nim(17, 10) Warning: hello; Ty1 is deprecated [Deprecated]
+  nimout: '''
-tmodule1.nim(20, 8) Warning: aVar is deprecated [Deprecated]
+tmodule1.nim(21, 8) Warning: goodbye; importme is deprecated [Deprecated]
-tmodule1.nim(22, 3) Warning: aProc is deprecated [Deprecated]
+tmodule1.nim(24, 10) Warning: Ty is deprecated [Deprecated]
-tmodule1.nim(23, 3) Warning: hello; aProc1 is deprecated [Deprecated]
+tmodule1.nim(27, 10) Warning: hello; Ty1 is deprecated [Deprecated]
+tmodule1.nim(30, 8) Warning: aVar is deprecated [Deprecated]
+tmodule1.nim(32, 3) Warning: aProc is deprecated [Deprecated]
+tmodule1.nim(33, 3) Warning: hello; aProc1 is deprecated [Deprecated]
 '''
 """
 
+
+
+
+
+
+
+# line 20
 import importme
 
 block:

tests/modules/tselfimport.nim

@@ -1,5 +1,5 @@
 discard """
-  errormsg: "A module cannot import itself"
+  errormsg: "module 'tselfimport' cannot import itself"
   file: "tselfimport.nim"
   line: 7
 """

tests/pragmas/mused2a.nim

@@ -0,0 +1,23 @@
+import std/strutils
+
+from std/os import fileExists
+
+import std/typetraits as typetraits2
+from std/setutils import complement
+
+
+
+
+
+proc fn1() = discard
+proc fn2*() = discard
+
+
+let fn4 = 0
+let fn5* = 0
+
+
+const fn7 = 0
+const fn8* = 0
+
+type T1 = object

tests/pragmas/mused2b.nim

@@ -0,0 +1,3 @@
+import mused2c
+export mused2c
+

tests/pragmas/mused2c.nim

@@ -0,0 +1 @@
+proc baz*() = discard

tests/pragmas/tused2.nim

@@ -0,0 +1,46 @@
+discard """
+  matrix: "--hint:all:off --hint:XDeclaredButNotUsed --path:."
+  joinable: false
+  nimoutFull: true
+  nimout: '''
+mused2a.nim(12, 6) Hint: 'fn1' is declared but not used [XDeclaredButNotUsed]
+mused2a.nim(16, 5) Hint: 'fn4' is declared but not used [XDeclaredButNotUsed]
+mused2a.nim(20, 7) Hint: 'fn7' is declared but not used [XDeclaredButNotUsed]
+mused2a.nim(23, 6) Hint: 'T1' is declared but not used [XDeclaredButNotUsed]
+mused2a.nim(1, 11) Warning: imported and not used: 'strutils' [UnusedImport]
+mused2a.nim(3, 9) Warning: imported and not used: 'os' [UnusedImport]
+mused2a.nim(5, 23) Warning: imported and not used: 'typetraits2' [UnusedImport]
+mused2a.nim(6, 9) Warning: imported and not used: 'setutils' [UnusedImport]
+tused2.nim(42, 8) Warning: imported and not used: 'mused2a' [UnusedImport]
+tused2.nim(45, 11) Warning: imported and not used: 'strutils' [UnusedImport]
+'''
+"""
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+# line 40
+
+import mused2a
+import mused2b
+
+import std/strutils
+baz()

tests/statictypes/tstatictypes.nim

@@ -271,7 +271,7 @@ block:
   fails(foo)
 
 
-import macros, tables
+import tables
 
 var foo{.compileTime.} = [
   "Foo",