Refactor and doc package handling, module name mangling (#19821)

* Refactor and doc package handling, module name mangling

* Consolidate, de-duplicate and extend package handling
* Alter how duplicate module names of a package are handled
* Alter how module names are mangled
* Fix crash when another package is named 'stdlib' (test case added)
* Doc what defines a package in the manual

Modules with duplicate names within a package used to be given 'fake'
packages to resolve conflicts. That prevented the ability to discern if
a module belonged to the current project package or a foreign package.
They now have the proper package owner and the names are mangled in a
consistent manner to prevent codegen clashes.

All module names are now mangled the same. Stdlib was treated special
before, but now it is same as any other package. This fixes a crash
when a foreign package is named 'stdlib'.

Module mangling is altered for both file paths and symbols used by the
backends.

Removed an unused module name to package mapping that may have been
intended for IC. The mapping was removed because it wasn't being used
and was complicating the issue of package modules with duplicate names
not having the proper package owner assigned.

* Fix some tests

* Refactor `packagehandling`

* Remove `packagehandling.withPackageName` and its uses
* Move module path mangling from `packagehandling` to `modulepaths`
* Move `options.toRodFile` to `ic` to break import cycle

* Changed import style to match preferred style

Co-authored-by: quantimnot <quantimnot@users.noreply.github.com>
(cherry picked from commit d30c6419a0)

compiler/ast.nim

@@ -1105,21 +1105,6 @@ proc getPIdent*(a: PNode): PIdent {.inline.} =
   of nkIdent: a.ident
   else: nil
 
-proc getnimblePkg*(a: PSym): PSym =
-  result = a
-  while result != nil:
-    case result.kind
-    of skModule:
-      result = result.owner
-      assert result.kind == skPackage
-    of skPackage:
-      if result.owner == nil:
-        break
-      else:
-        result = result.owner
-    else:
-      assert false, $result.kind
-
 const
   moduleShift = when defined(cpu32): 20 else: 24
 
@@ -1164,13 +1149,7 @@ when false:
     assert dest.ItemId.item <= src.ItemId.item
     dest = src
 
-proc getnimblePkgId*(a: PSym): int =
-  let b = a.getnimblePkg
-  result = if b == nil: -1 else: b.id
-
 var ggDebug* {.deprecated.}: bool ## convenience switch for trying out things
-#var
-#  gMainPackageId*: int
 
 proc isCallExpr*(n: PNode): bool =
   result = n.kind in nkCallKinds

compiler/cgen.nim

@@ -15,7 +15,7 @@ import
   ccgutils, os, ropes, math, passes, wordrecg, treetab, cgmeth,
   rodutils, renderer, cgendata, aliases,
   lowerings, tables, sets, ndi, lineinfos, pathutils, transf,
-  injectdestructors, astmsgs
+  injectdestructors, astmsgs, modulepaths
 
 when not defined(leanCompiler):
   import spawn, semparallel
@@ -1302,17 +1302,19 @@ proc getFileHeader(conf: ConfigRef; cfile: Cfile): Rope =
   if conf.hcrOn: result.add("#define NIM_HOT_CODE_RELOADING\L")
   addNimDefines(result, conf)
 
-proc getSomeNameForModule(m: PSym): Rope =
-  assert m.kind == skModule
-  assert m.owner.kind == skPackage
-  if {sfSystemModule, sfMainModule} * m.flags == {}:
-    result = m.owner.name.s.mangle.rope
-    result.add "_"
-  result.add m.name.s.mangle
+proc getSomeNameForModule(conf: ConfigRef, filename: AbsoluteFile): Rope =
+  ## Returns a mangled module name.
+  result.add mangleModuleName(conf, filename).mangle
+
+proc getSomeNameForModule(m: BModule): Rope =
+  ## Returns a mangled module name.
+  assert m.module.kind == skModule
+  assert m.module.owner.kind == skPackage
+  result.add mangleModuleName(m.g.config, m.filename).mangle
 
 proc getSomeInitName(m: BModule, suffix: string): Rope =
   if not m.hcrOn:
-    result = getSomeNameForModule(m.module)
+    result = getSomeNameForModule(m)
   result.add suffix
 
 proc getInitName(m: BModule): Rope =
@@ -1551,11 +1553,11 @@ proc genMainProc(m: BModule) =
 proc registerInitProcs*(g: BModuleList; m: PSym; flags: set[ModuleBackendFlag]) =
   ## Called from the IC backend.
   if HasDatInitProc in flags:
-    let datInit = getSomeNameForModule(m) & "DatInit000"
+    let datInit = getSomeNameForModule(g.config, g.config.toFullPath(m.info.fileIndex).AbsoluteFile) & "DatInit000"
     g.mainModProcs.addf("N_LIB_PRIVATE N_NIMCALL(void, $1)(void);$N", [datInit])
     g.mainDatInit.addf("\t$1();$N", [datInit])
   if HasModuleInitProc in flags:
-    let init = getSomeNameForModule(m) & "Init000"
+    let init = getSomeNameForModule(g.config, g.config.toFullPath(m.info.fileIndex).AbsoluteFile) & "Init000"
     g.mainModProcs.addf("N_LIB_PRIVATE N_NIMCALL(void, $1)(void);$N", [init])
     let initCall = "\t$1();$N" % [init]
     if sfMainModule in m.flags:
@@ -1936,7 +1938,7 @@ proc getCFile(m: BModule): AbsoluteFile =
       if m.compileToCpp: ".nim.cpp"
       elif m.config.backend == backendObjc or sfCompileToObjc in m.module.flags: ".nim.m"
       else: ".nim.c"
-  result = changeFileExt(completeCfilePath(m.config, withPackageName(m.config, m.cfilename)), ext)
+  result = changeFileExt(completeCfilePath(m.config, mangleModuleName(m.config, m.cfilename).AbsoluteFile), ext)
 
 when false:
   proc myOpenCached(graph: ModuleGraph; module: PSym, rd: PRodReader): PPassContext =

compiler/docgen.nim

@@ -16,7 +16,7 @@ import
   packages/docutils/rst, packages/docutils/rstgen,
   json, xmltree, trees, types,
   typesrenderer, astalgo, lineinfos, intsets,
-  pathutils, tables, nimpaths, renderverbatim, osproc
+  pathutils, tables, nimpaths, renderverbatim, osproc, packages
 import packages/docutils/rstast except FileIndex, TLineInfo
 
 from uri import encodeUrl
@@ -395,9 +395,6 @@ proc getPlainDocstring(n: PNode): string =
       result = getPlainDocstring(n[i])
       if result.len > 0: return
 
-proc belongsToPackage(conf: ConfigRef; module: PSym): bool =
-  result = module.kind == skModule and module.getnimblePkgId == conf.mainPackageId
-
 proc externalDep(d: PDoc; module: PSym): string =
   if optWholeProject in d.conf.globalOptions or d.conf.docRoot.len > 0:
     let full = AbsoluteFile toFullPath(d.conf, FileIndex module.position)
@@ -453,7 +450,7 @@ proc nodeToHighlightedHtml(d: PDoc; n: PNode; result: var string;
               "\\spanIdentifier{$1}", [escLit, procLink])
       elif s != nil and s.kind in {skType, skVar, skLet, skConst} and
            sfExported in s.flags and s.owner != nil and
-           belongsToPackage(d.conf, s.owner) and d.target == outHtml:
+           belongsToProjectPackage(d.conf, s.owner) and d.target == outHtml:
         let external = externalDep(d, s.owner)
         result.addf "<a href=\"$1#$2\"><span class=\"Identifier\">$3</span></a>",
           [changeFileExt(external, "html"), literal,
@@ -1041,7 +1038,7 @@ proc traceDeps(d: PDoc, it: PNode) =
     for x in it[2]:
       a[2] = x
       traceDeps(d, a)
-  elif it.kind == nkSym and belongsToPackage(d.conf, it.sym):
+  elif it.kind == nkSym and belongsToProjectPackage(d.conf, it.sym):
     let external = externalDep(d, it.sym)
     if d.section[k].finalMarkup != "": d.section[k].finalMarkup.add(", ")
     dispA(d.conf, d.section[k].finalMarkup,
@@ -1051,7 +1048,7 @@ proc traceDeps(d: PDoc, it: PNode) =
 
 proc exportSym(d: PDoc; s: PSym) =
   const k = exportSection
-  if s.kind == skModule and belongsToPackage(d.conf, s):
+  if s.kind == skModule and belongsToProjectPackage(d.conf, s):
     let external = externalDep(d, s)
     if d.section[k].finalMarkup != "": d.section[k].finalMarkup.add(", ")
     dispA(d.conf, d.section[k].finalMarkup,
@@ -1060,7 +1057,7 @@ proc exportSym(d: PDoc; s: PSym) =
                  changeFileExt(external, "html")])
   elif s.kind != skModule and s.owner != nil:
     let module = originatingModule(s)
-    if belongsToPackage(d.conf, module):
+    if belongsToProjectPackage(d.conf, module):
       let
         complexSymbol = complexName(s.kind, s.ast, s.name.s)
         symbolOrId = d.newUniquePlainSymbol(complexSymbol)

compiler/docgen2.nim

@@ -11,7 +11,7 @@
 # semantic checking.
 
 import
-  options, ast, msgs, passes, docgen, lineinfos, pathutils
+  options, ast, msgs, passes, docgen, lineinfos, pathutils, packages
 
 from modulegraphs import ModuleGraph, PPassContext
 
@@ -23,7 +23,7 @@ type
   PGen = ref TGen
 
 proc shouldProcess(g: PGen): bool =
-  (optWholeProject in g.doc.conf.globalOptions and g.module.getnimblePkgId == g.doc.conf.mainPackageId) or
+  (optWholeProject in g.doc.conf.globalOptions and g.doc.conf.belongsToProjectPackage(g.module)) or
       sfMainModule in g.module.flags or g.config.projectMainIdx == g.module.info.fileIndex
 
 template closeImpl(body: untyped) {.dirty.} =

compiler/extccomp.nim

@@ -12,7 +12,7 @@
 # from a lineinfos file, to provide generalized procedures to compile
 # nim files.
 
-import ropes, platform, condsyms, options, msgs, lineinfos, pathutils
+import ropes, platform, condsyms, options, msgs, lineinfos, pathutils, modulepaths
 
 import std/[os, strutils, osproc, sha1, streams, sequtils, times, strtabs, json, jsonutils, sugar]
 
@@ -367,6 +367,7 @@ proc initVars*(conf: ConfigRef) =
 
 proc completeCfilePath*(conf: ConfigRef; cfile: AbsoluteFile,
                         createSubDir: bool = true): AbsoluteFile =
+  ## Generate the absolute file path to the generated modules.
   result = completeGeneratedFilePath(conf, cfile, createSubDir)
 
 proc toObjFile*(conf: ConfigRef; filename: AbsoluteFile): AbsoluteFile =
@@ -377,7 +378,7 @@ proc addFileToCompile*(conf: ConfigRef; cf: Cfile) =
   conf.toCompile.add(cf)
 
 proc addLocalCompileOption*(conf: ConfigRef; option: string; nimfile: AbsoluteFile) =
-  let key = completeCfilePath(conf, withPackageName(conf, nimfile)).string
+  let key = completeCfilePath(conf, mangleModuleName(conf, nimfile).AbsoluteFile).string
   var value = conf.cfileSpecificOptions.getOrDefault(key)
   if strutils.find(value, option, 0) < 0:
     addOpt(value, option)
@@ -640,7 +641,7 @@ proc footprint(conf: ConfigRef; cfile: Cfile): SecureHash =
 proc externalFileChanged(conf: ConfigRef; cfile: Cfile): bool =
   if conf.backend == backendJs: return false # pre-existing behavior, but not sure it's good
 
-  let hashFile = toGeneratedFile(conf, conf.withPackageName(cfile.cname), "sha1")
+  let hashFile = toGeneratedFile(conf, conf.mangleModuleName(cfile.cname).AbsoluteFile, "sha1")
   let currentHash = footprint(conf, cfile)
   var f: File
   if open(f, hashFile.string, fmRead):
@@ -848,9 +849,9 @@ proc hcrLinkTargetName(conf: ConfigRef, objFile: string, isMain = false): Absolu
 proc displayProgressCC(conf: ConfigRef, path, compileCmd: string): string =
   if conf.hasHint(hintCC):
     if optListCmd in conf.globalOptions or conf.verbosity > 1:
-      result = MsgKindToStr[hintCC] % (demanglePackageName(path.splitFile.name) & ": " & compileCmd)
+      result = MsgKindToStr[hintCC] % (demangleModuleName(path.splitFile.name) & ": " & compileCmd)
     else:
-      result = MsgKindToStr[hintCC] % demanglePackageName(path.splitFile.name)
+      result = MsgKindToStr[hintCC] % demangleModuleName(path.splitFile.name)
 
 proc callCCompiler*(conf: ConfigRef) =
   var

compiler/ic/cbackend.nim

@@ -21,7 +21,7 @@
 import std/packedsets, algorithm, tables
 
 import ".."/[ast, options, lineinfos, modulegraphs, cgendata, cgen,
-  pathutils, extccomp, msgs]
+  pathutils, extccomp, msgs, modulepaths]
 
 import packed_ast, ic, dce, rodfiles
 
@@ -61,7 +61,8 @@ proc addFileToLink(config: ConfigRef; m: PSym) =
       if config.backend == backendCpp: ".nim.cpp"
       elif config.backend == backendObjc: ".nim.m"
       else: ".nim.c"
-  let cfile = changeFileExt(completeCfilePath(config, withPackageName(config, filename)), ext)
+  let cfile = changeFileExt(completeCfilePath(config,
+                            mangleModuleName(config, filename).AbsoluteFile), ext)
   let objFile = completeCfilePath(config, toObjFile(config, cfile))
   if fileExists(objFile):
     var cf = Cfile(nimname: m.name.s, cname: cfile,

compiler/ic/ic.nim

@@ -10,7 +10,7 @@
 import hashes, tables, intsets, std/sha1
 import packed_ast, bitabs, rodfiles
 import ".." / [ast, idents, lineinfos, msgs, ropes, options,
-  pathutils, condsyms]
+  pathutils, condsyms, packages, modulepaths]
 #import ".." / [renderer, astalgo]
 from os import removeFile, isAbsolute
 
@@ -548,6 +548,10 @@ proc loadError(err: RodFileError; filename: AbsoluteFile; config: ConfigRef;) =
     rawMessage(config, warnCannotOpenFile, filename.string & " reason: " & $err)
     #echo "Error: ", $err, " loading file: ", filename.string
 
+proc toRodFile*(conf: ConfigRef; f: AbsoluteFile; ext = RodExt): AbsoluteFile =
+  result = changeFileExt(completeGeneratedFilePath(conf,
+    mangleModuleName(conf, f).AbsoluteFile), ext)
+
 proc loadRodFile*(filename: AbsoluteFile; m: var PackedModule; config: ConfigRef;
                   ignoreConfig = false): RodFileError =
   var f = rodfiles.open(filename.string)
@@ -927,17 +931,6 @@ proc loadType(c: var PackedDecoder; g: var PackedModuleGraph; thisModule: int; t
       result = g[si].types[t.item]
     assert result.itemId.item > 0
 
-proc newPackage(config: ConfigRef; cache: IdentCache; fileIdx: FileIndex): PSym =
-  let filename = AbsoluteFile toFullPath(config, fileIdx)
-  let name = getIdent(cache, splitFile(filename).name)
-  let info = newLineInfo(fileIdx, 1, 1)
-  let
-    pck = getPackageName(config, filename.string)
-    pck2 = if pck.len > 0: pck else: "unknown"
-    pack = getIdent(cache, pck2)
-  result = newSym(skPackage, getIdent(cache, pck2),
-    ItemId(module: PackageModuleId, item: int32(fileIdx)), nil, info)
-
 proc setupLookupTables(g: var PackedModuleGraph; conf: ConfigRef; cache: IdentCache;
                        fileIdx: FileIndex; m: var LoadedModule) =
   m.iface = initTable[PIdent, seq[PackedItemId]]()
@@ -965,7 +958,7 @@ proc setupLookupTables(g: var PackedModuleGraph; conf: ConfigRef; cache: IdentCa
                   name: getIdent(cache, splitFile(filename).name),
                   info: newLineInfo(fileIdx, 1, 1),
                   position: int(fileIdx))
-  m.module.owner = newPackage(conf, cache, fileIdx)
+  m.module.owner = getPackage(conf, cache, fileIdx)
   m.module.flags = m.fromDisk.moduleFlags
 
 proc loadToReplayNodes(g: var PackedModuleGraph; conf: ConfigRef; cache: IdentCache;

compiler/modulegraphs.nim

@@ -12,7 +12,7 @@
 ## or stored in a rod-file.
 
 import intsets, tables, hashes, md5, sequtils
-import ast, astalgo, options, lineinfos,idents, btrees, ropes, msgs, pathutils
+import ast, astalgo, options, lineinfos,idents, btrees, ropes, msgs, pathutils, packages
 import ic / [packed_ast, ic]
 
 type
@@ -64,7 +64,6 @@ type
 
     startupPackedConfig*: PackedConfig
     packageSyms*: TStrTable
-    modulesPerPackage*: Table[ItemId, TStrTable]
     deps*: IntSet # the dependency graph or potentially its transitive closure.
     importDeps*: Table[FileIndex, seq[FileIndex]] # explicit import module dependencies
     suggestMode*: bool # whether we are in nimsuggest mode or not.
@@ -653,3 +652,18 @@ iterator suggestErrorsIter*(g: ModuleGraph): Suggest =
   for xs in g.suggestErrors.values:
     for x in xs:
       yield x
+
+proc getPackage*(graph: ModuleGraph; fileIdx: FileIndex): PSym =
+  ## Returns a package symbol for yet to be defined module for fileIdx.
+  ## The package symbol is added to the graph if it doesn't exist.
+  let pkgSym = getPackage(graph.config, graph.cache, fileIdx)
+  # check if the package is already in the graph
+  result = graph.packageSyms.strTableGet(pkgSym.name)
+  if result == nil:
+     # the package isn't in the graph, so create and add it
+    result = pkgSym
+    graph.packageSyms.strTableAdd(pkgSym)
+
+func belongsToStdlib*(graph: ModuleGraph, sym: PSym): bool =
+  ## Check if symbol belongs to the 'stdlib' package.
+  sym.getPackageSymbol.getPackageId == graph.systemModule.getPackageId

compiler/modulepaths.nim

@@ -10,100 +10,6 @@
 import ast, renderer, strutils, msgs, options, idents, os, lineinfos,
   pathutils
 
-when false:
-  const
-    considerParentDirs = not defined(noParentProjects)
-    considerNimbleDirs = not defined(noNimbleDirs)
-
-  proc findInNimbleDir(pkg, subdir, dir: string): string =
-    var best = ""
-    var bestv = ""
-    for k, p in os.walkDir(dir, relative=true):
-      if k == pcDir and p.len > pkg.len+1 and
-          p[pkg.len] == '-' and p.startsWith(pkg):
-        let (_, a, _) = getPathVersionChecksum(p)
-        if bestv.len == 0 or bestv < a:
-          bestv = a
-          best = dir / p
-
-    if best.len > 0:
-      var f: File
-      if open(f, best / changeFileExt(pkg, ".nimble-link")):
-        # the second line contains what we're interested in, see:
-        # https://github.com/nim-lang/nimble#nimble-link
-        var override = ""
-        discard readLine(f, override)
-        discard readLine(f, override)
-        close(f)
-        if not override.isAbsolute():
-          best = best / override
-        else:
-          best = override
-    let f = if subdir.len == 0: pkg else: subdir
-    let res = addFileExt(best / f, "nim")
-    if best.len > 0 and fileExists(res):
-      result = res
-
-when false:
-  proc resolveDollar(project, source, pkg, subdir: string; info: TLineInfo): string =
-    template attempt(a) =
-      let x = addFileExt(a, "nim")
-      if fileExists(x): return x
-
-    case pkg
-    of "stdlib":
-      if subdir.len == 0:
-        return options.libpath
-      else:
-        for candidate in stdlibDirs:
-          attempt(options.libpath / candidate / subdir)
-    of "root":
-      let root = project.splitFile.dir
-      if subdir.len == 0:
-        return root
-      else:
-        attempt(root / subdir)
-    else:
-      when considerParentDirs:
-        var p = parentDir(source.splitFile.dir)
-        # support 'import $karax':
-        let f = if subdir.len == 0: pkg else: subdir
-
-        while p.len > 0:
-          let dir = p / pkg
-          if dirExists(dir):
-            attempt(dir / f)
-            # 2nd attempt: try to use 'karax/karax'
-            attempt(dir / pkg / f)
-            # 3rd attempt: try to use 'karax/src/karax'
-            attempt(dir / "src" / f)
-            attempt(dir / "src" / pkg / f)
-          p = parentDir(p)
-
-      when considerNimbleDirs:
-        if not options.gNoNimblePath:
-          var nimbleDir = getEnv("NIMBLE_DIR")
-          if nimbleDir.len == 0: nimbleDir = getHomeDir() / ".nimble"
-          result = findInNimbleDir(pkg, subdir, nimbleDir / "pkgs")
-          if result.len > 0: return result
-          when not defined(windows):
-            result = findInNimbleDir(pkg, subdir, "/opt/nimble/pkgs")
-            if result.len > 0: return result
-
-  proc scriptableImport(pkg, sub: string; info: TLineInfo): string =
-    resolveDollar(gProjectFull, info.toFullPath(), pkg, sub, info)
-
-  proc lookupPackage(pkg, subdir: PNode): string =
-    let sub = if subdir != nil: renderTree(subdir, {renderNoComments}).replace(" ") else: ""
-    case pkg.kind
-    of nkStrLit, nkRStrLit, nkTripleStrLit:
-      result = scriptableImport(pkg.strVal, sub, pkg.info)
-    of nkIdent:
-      result = scriptableImport(pkg.ident.s, sub, pkg.info)
-    else:
-      localError(pkg.info, "package name must be an identifier or string literal")
-      result = ""
-
 proc getModuleName*(conf: ConfigRef; n: PNode): string =
   # This returns a short relative module name without the nim extension
   # e.g. like "system", "importer" or "somepath/module"
@@ -163,3 +69,18 @@ proc checkModuleName*(conf: ConfigRef; n: PNode; doLocalError=true): FileIndex =
     result = InvalidFileIdx
   else:
     result = fileInfoIdx(conf, fullPath)
+
+proc mangleModuleName*(conf: ConfigRef; path: AbsoluteFile): string =
+  ## Mangle a relative module path to avoid path and symbol collisions.
+  ##
+  ## Used by backends that need to generate intermediary files from Nim modules.
+  ## This is needed because the compiler uses a flat cache file hierarchy.
+  ##
+  ## Example:
+  ## `foo-#head/../bar` becomes `@foo-@hhead@s..@sbar`
+  "@m" & relativeTo(path, conf.projectPath).string.multiReplace(
+    {$os.DirSep: "@s", $os.AltSep: "@s", "#": "@h", "@": "@@", ":": "@c"})
+
+proc demangleModuleName*(path: string): string =
+  ## Demangle a relative module path.
+  result = path.multiReplace({"@@": "@", "@h": "#", "@s": "/", "@m": "", "@c": ":"})

compiler/modules.nim

@@ -12,7 +12,7 @@
 import
   ast, astalgo, magicsys, msgs, options,
   idents, lexer, passes, syntaxes, llstream, modulegraphs,
-  lineinfos, pathutils, tables
+  lineinfos, pathutils, tables, packages
 
 import ic / replayer
 
@@ -22,56 +22,11 @@ proc resetSystemArtifacts*(g: ModuleGraph) =
 template getModuleIdent(graph: ModuleGraph, filename: AbsoluteFile): PIdent =
   getIdent(graph.cache, splitFile(filename).name)
 
-template packageId(): untyped {.dirty.} = ItemId(module: PackageModuleId, item: int32(fileIdx))
-
-proc getPackage(graph: ModuleGraph; fileIdx: FileIndex): PSym =
-  ## returns package symbol (skPackage) for yet to be defined module for fileIdx
-  let filename = AbsoluteFile toFullPath(graph.config, fileIdx)
-  let name = getModuleIdent(graph, filename)
-  let info = newLineInfo(fileIdx, 1, 1)
-  let
-    pck = getPackageName(graph.config, filename.string)
-    pck2 = if pck.len > 0: pck else: "unknown"
-    pack = getIdent(graph.cache, pck2)
-  result = graph.packageSyms.strTableGet(pack)
-  if result == nil:
-    result = newSym(skPackage, getIdent(graph.cache, pck2), packageId(), nil, info)
-    #initStrTable(packSym.tab)
-    graph.packageSyms.strTableAdd(result)
-  else:
-    let modules = graph.modulesPerPackage.getOrDefault(result.itemId)
-    let existing = if modules.data.len > 0: strTableGet(modules, name) else: nil
-    if existing != nil and existing.info.fileIndex != info.fileIndex:
-      when false:
-        # we used to produce an error:
-        localError(graph.config, info,
-          "module names need to be unique per Nimble package; module clashes with " &
-            toFullPath(graph.config, existing.info.fileIndex))
-      else:
-        # but starting with version 0.20 we now produce a fake Nimble package instead
-        # to resolve the conflicts:
-        let pck3 = fakePackageName(graph.config, filename)
-        # this makes the new `result`'s owner be the original `result`
-        result = newSym(skPackage, getIdent(graph.cache, pck3), packageId(), result, info)
-        #initStrTable(packSym.tab)
-        graph.packageSyms.strTableAdd(result)
-
 proc partialInitModule(result: PSym; graph: ModuleGraph; fileIdx: FileIndex; filename: AbsoluteFile) =
   let packSym = getPackage(graph, fileIdx)
   result.owner = packSym
   result.position = int fileIdx
 
-  #initStrTable(result.tab(graph))
-  when false:
-    strTableAdd(result.tab, result) # a module knows itself
-    # This is now implemented via
-    #   c.moduleScope.addSym(module) # a module knows itself
-    # in sem.nim, around line 527
-
-  if graph.modulesPerPackage.getOrDefault(packSym.itemId).data.len == 0:
-    graph.modulesPerPackage[packSym.itemId] = newStrTable()
-  graph.modulesPerPackage[packSym.itemId].strTableAdd(result)
-
 proc newModule(graph: ModuleGraph; fileIdx: FileIndex): PSym =
   let filename = AbsoluteFile toFullPath(graph.config, fileIdx)
   # We cannot call ``newSym`` here, because we have to circumvent the ID
@@ -133,7 +88,7 @@ proc importModule*(graph: ModuleGraph; s: PSym, fileIdx: FileIndex): PSym =
   #  localError(result.info, errAttemptToRedefine, result.name.s)
   # restore the notes for outer module:
   graph.config.notes =
-    if s.getnimblePkgId == graph.config.mainPackageId or isDefined(graph.config, "booting"): graph.config.mainPackageNotes
+    if graph.config.belongsToProjectPackage(s) or isDefined(graph.config, "booting"): graph.config.mainPackageNotes
     else: graph.config.foreignPackageNotes
 
 proc includeModule*(graph: ModuleGraph; s: PSym, fileIdx: FileIndex): PNode =
@@ -168,7 +123,7 @@ proc compileProject*(graph: ModuleGraph; projectFileIdx = InvalidFileIdx) =
   conf.projectMainIdx2 = projectFile
 
   let packSym = getPackage(graph, projectFile)
-  graph.config.mainPackageId = packSym.getnimblePkgId
+  graph.config.mainPackageId = packSym.getPackageId
   graph.importStack.add projectFile
 
   if projectFile == systemFileIdx:

compiler/options.nim

@@ -802,6 +802,8 @@ proc toGeneratedFile*(conf: ConfigRef; path: AbsoluteFile,
 
 proc completeGeneratedFilePath*(conf: ConfigRef; f: AbsoluteFile,
                                 createSubDir: bool = true): AbsoluteFile =
+  ## Return an absolute path of a generated intermediary file.
+  ## Optionally creates the cache directory if `createSubDir` is `true`.
   let subdir = getNimcacheDir(conf)
   if createSubDir:
     try:
@@ -809,11 +811,6 @@ proc completeGeneratedFilePath*(conf: ConfigRef; f: AbsoluteFile,
     except OSError:
       conf.quitOrRaise "cannot create directory: " & subdir.string
   result = subdir / RelativeFile f.string.splitPath.tail
-  #echo "completeGeneratedFilePath(", f, ") = ", result
-
-proc toRodFile*(conf: ConfigRef; f: AbsoluteFile; ext = RodExt): AbsoluteFile =
-  result = changeFileExt(completeGeneratedFilePath(conf,
-    withPackageName(conf, f)), ext)
 
 proc rawFindFile(conf: ConfigRef; f: RelativeFile; suppressStdlib: bool): AbsoluteFile =
   for it in conf.searchPaths:

compiler/packagehandling.nim

@@ -37,24 +37,7 @@ proc getNimbleFile*(conf: ConfigRef; path: string): string =
 proc getPackageName*(conf: ConfigRef; path: string): string =
   ## returns nimble package name, e.g.: `cligen`
   let path = getNimbleFile(conf, path)
-  result = path.splitFile.name
-
-proc fakePackageName*(conf: ConfigRef; path: AbsoluteFile): string =
-  # Convert `path` so that 2 modules with same name
-  # in different directory get different name and they can be
-  # placed in a directory.
-  # foo-#head/../bar becomes @foo-@hhead@s..@sbar
-  result = "@m" & relativeTo(path, conf.projectPath).string.multiReplace(
-    {$os.DirSep: "@s", $os.AltSep: "@s", "#": "@h", "@": "@@", ":": "@c"})
-
-proc demanglePackageName*(path: string): string =
-  result = path.multiReplace({"@@": "@", "@h": "#", "@s": "/", "@m": "", "@c": ":"})
-
-proc withPackageName*(conf: ConfigRef; path: AbsoluteFile): AbsoluteFile =
-  let x = getPackageName(conf, path.string)
-  let (p, file, ext) = path.splitFile
-  if x == "stdlib":
-    # Hot code reloading now relies on 'stdlib_system' names etc.
-    result = p / RelativeFile((x & '_' & file) & ext)
+  if path.len > 0:
+    return path.splitFile.name
   else:
-    result = p / RelativeFile(fakePackageName(conf, path))
+    return "unknown"

compiler/packages.nim

@@ -0,0 +1,49 @@
+#
+#
+#           The Nim Compiler
+#        (c) Copyright 2022 Andreas Rumpf
+#
+#    See the file "copying.txt", included in this
+#    distribution, for details about the copyright.
+#
+
+## Package related procs.
+## 
+## See Also:
+## * `packagehandling` for package path handling
+## * `modulegraphs.getPackage`
+## * `modulegraphs.belongsToStdlib`
+
+import "." / [options, ast, lineinfos, idents, pathutils, msgs]
+
+proc getPackage*(conf: ConfigRef; cache: IdentCache; fileIdx: FileIndex): PSym =
+  ## Return a new package symbol.
+  ## 
+  ## See Also:
+  ## * `modulegraphs.getPackage`
+  let
+    filename = AbsoluteFile toFullPath(conf, fileIdx)
+    name = getIdent(cache, splitFile(filename).name)
+    info = newLineInfo(fileIdx, 1, 1)
+    pkgName = getPackageName(conf, filename.string)
+    pkgIdent = getIdent(cache, pkgName)
+  newSym(skPackage, pkgIdent, ItemId(module: PackageModuleId, item: int32(fileIdx)), nil, info)
+
+func getPackageSymbol*(sym: PSym): PSym =
+  ## Return the owning package symbol.
+  assert sym != nil
+  result = sym
+  while result.kind != skPackage:
+    result = result.owner
+    assert result != nil, repr(sym.info)
+
+func getPackageId*(sym: PSym): int =
+  ## Return the owning package ID.
+  sym.getPackageSymbol.id
+
+func belongsToProjectPackage*(conf: ConfigRef, sym: PSym): bool =
+  ## Return whether the symbol belongs to the project's package.
+  ## 
+  ## See Also:
+  ## * `modulegraphs.belongsToStdlib`
+  conf.mainPackageId == sym.getPackageId

compiler/passes.nim

@@ -14,7 +14,7 @@ import
   options, ast, llstream, msgs,
   idents,
   syntaxes, modulegraphs, reorder,
-  lineinfos, pathutils, std/sha1
+  lineinfos, pathutils, std/sha1, packages
 
 
 type
@@ -102,7 +102,7 @@ const
 
 proc prepareConfigNotes(graph: ModuleGraph; module: PSym) =
   # don't be verbose unless the module belongs to the main package:
-  if module.getnimblePkgId == graph.config.mainPackageId:
+  if graph.config.belongsToProjectPackage(module):
     graph.config.notes = graph.config.mainPackageNotes
   else:
     if graph.config.mainPackageNotes == {}: graph.config.mainPackageNotes = graph.config.notes
@@ -112,12 +112,6 @@ proc moduleHasChanged*(graph: ModuleGraph; module: PSym): bool {.inline.} =
   result = true
   #module.id >= 0 or isDefined(graph.config, "nimBackendAssumesChange")
 
-proc partOfStdlib(x: PSym): bool =
-  var it = x.owner
-  while it != nil and it.kind == skPackage and it.owner != nil:
-    it = it.owner
-  result = it != nil and it.name.s == "stdlib"
-
 proc processModule*(graph: ModuleGraph; module: PSym; idgen: IdGenerator;
                     stream: PLLStream): bool {.discardable.} =
   if graph.stopCompile(): return true
@@ -144,7 +138,7 @@ proc processModule*(graph: ModuleGraph; module: PSym; idgen: IdGenerator;
   while true:
     openParser(p, fileIdx, s, graph.cache, graph.config)
 
-    if not partOfStdlib(module) or module.name.s == "distros":
+    if not belongsToStdlib(graph, module) or (belongsToStdlib(graph, module) and module.name.s == "distros"):
       # XXX what about caching? no processing then? what if I change the
       # modules to include between compilation runs? we'd need to track that
       # in ROD files. I think we should enable this feature only

compiler/pragmas.nim

@@ -635,12 +635,13 @@ proc pragmaLine(c: PContext, n: PNode) =
     n.info = getInfoContext(c.config, -1)
 
 proc processPragma(c: PContext, n: PNode, i: int) =
+  ## Create and add a new custom pragma `{.pragma: name.}` node to the module's context.
   let it = n[i]
   if it.kind notin nkPragmaCallKinds and it.safeLen == 2: invalidPragma(c, n)
   elif it.safeLen != 2 or it[0].kind != nkIdent or it[1].kind != nkIdent:
     invalidPragma(c, n)
 
-  var userPragma = newSym(skTemplate, it[1].ident, nextSymId(c.idgen), nil, it.info, c.config.options)
+  var userPragma = newSym(skTemplate, it[1].ident, nextSymId(c.idgen), c.module, it.info, c.config.options)
   userPragma.ast = newTreeI(nkPragma, n.info, n.sons[i+1..^1])
   strTableAdd(c.userPragmas, userPragma)
 

doc/manual.rst

@@ -6488,6 +6488,19 @@ iterator in which case the overloading resolution takes place:
   write(stdout, x) # not ambiguous: uses the module C's x
 
 
+Packages
+--------
+A collection of modules in a file tree with an ``identifier.nimble`` file in the
+root of the tree is called a Nimble package. A valid package name can only be a
+valid Nim identifier and thus its filename is ``identifier.nimble`` where
+``identifier`` is the desired package name. A module without a ``.nimble`` file
+is assigned the package identifier: `unknown`.
+
+The distinction between packages allows diagnostic compiler messages to be
+scoped to the current project's package vs foreign packages.
+
+
+
 Compiler Messages
 =================
 

tests/ccgbugs/tforward_decl_only.nim

@@ -1,7 +1,7 @@
 discard """
 ccodecheck: "\\i !@('struct tyObject_MyRefObject'[0-z]+' {')"
 ccodecheck: "\\i !@('mymoduleInit')"
-ccodecheck: "\\i @('mymoduleDatInit')"
+ccodecheck: "\\i @('atmmymoduledotnim_DatInit000')"
 output: "hello"
 """
 

tests/modules/a/module_name_clashes.nim

@@ -0,0 +1,8 @@
+# See `tmodule_name_clashes`
+
+import ../b/module_name_clashes
+type A* = object
+  b*: B
+
+proc print*(a: A) =
+  echo repr a

tests/modules/b/module_name_clashes.nim

@@ -0,0 +1,3 @@
+# See `tmodule_name_clashes`
+
+type B* = object

tests/modules/tmodule_name_clashes.nim

@@ -0,0 +1,16 @@
+discard """
+targets: "c"
+ccodecheck: "\\i @('atmaatsmodule_name_clashesdotnim_DatInit000')"
+ccodecheck: "\\i @('atmbatsmodule_name_clashesdotnim_DatInit000')"
+joinable: false
+"""
+
+# Test module name clashes within same package.
+# This was created to test that module symbol mangling functioned correctly
+# for the C backend when there are one or more modules with the same name in
+# a package, and more than one of them require module initialization procs.
+# I'm not sure of the simplest method to cause the init procs to be generated.
+
+import a/module_name_clashes
+
+print A()

tests/package/stdlib/system.nim

@@ -0,0 +1,2 @@
+# this module is part of tstdlib_name_not_special
+doAssert true

tests/package/tstdlib_name_not_special.nim

@@ -0,0 +1,3 @@
+# Test whether a another package named 'stdlib' can be imported and used.
+# This caused a crash in the past.
+import stdlib/system