bug fixes with sfMainModule, hints, mainPackageNotes, mainPackageId, hintSuccessX (#14555)

* SuccessX `out` now works with --compileOnly and jsonscript; fix bugs in jsonscript
* several bug fixes; eg: `nim doc lib/system/io` now is sane
* dummy edit to force docs CI

.github/workflows/ci_docs.yml

@@ -12,7 +12,7 @@ on:
       - 'tools/kochdocs.nim'
       - '.github/workflows/ci_docs.yml'
   pull_request:
-    # Run only on changes on these files
+    # Run only on changes on these files.
     paths:
       - 'compiler/docgen.nim'
       - 'compiler/renderverbatim.nim'

compiler/commands.nim

@@ -913,8 +913,6 @@ proc processSwitch*(switch, arg: string, pass: TCmdLinePass, info: TLineInfo;
     if strutils.find(switch, '.') >= 0: options.setConfigVar(conf, switch, arg)
     else: invalidCmdLineOption(conf, pass, switch, info)
 
-template gCmdLineInfo*(): untyped = newLineInfo(commandLineIdx, 1, 1)
-
 proc processCommand*(switch: string, pass: TCmdLinePass; config: ConfigRef) =
   var cmd, arg: string
   splitSwitch(config, switch, cmd, arg, pass, gCmdLineInfo)

compiler/extccomp.nim

@@ -1003,10 +1003,16 @@ proc writeJsonBuildInstructions*(conf: ConfigRef) =
   var buf = newStringOfCap(50)
 
   let jsonFile = conf.getNimcacheDir / RelativeFile(conf.projectName & ".json")
+  conf.jsonBuildFile = jsonFile
+  let output = conf.absOutFile
 
   var f: File
   if open(f, jsonFile.string, fmWrite):
-    lit "{\"compile\":[\L"
+    lit "{\L"
+    lit "\"outputFile\": "
+    str $output
+
+    lit ",\L\"compile\":[\L"
     cfiles(conf, f, buf, conf.toCompile, false)
     lit "],\L\"link\":[\L"
     var objfiles = ""
@@ -1014,7 +1020,7 @@ proc writeJsonBuildInstructions*(conf: ConfigRef) =
     linkfiles(conf, f, buf, objfiles, conf.toCompile, conf.externalToLink)
 
     lit "],\L\"linkcmd\": "
-    str getLinkCmd(conf, conf.absOutFile, objfiles)
+    str getLinkCmd(conf, output, objfiles)
 
     lit ",\L\"extraCmds\": "
     lit $(%* getExtraCmds(conf, conf.absOutFile))
@@ -1074,6 +1080,17 @@ proc runJsonBuildInstructions*(conf: ConfigRef; projectfile: AbsoluteFile) =
   let jsonFile = toGeneratedFile(conf, projectfile, "json")
   try:
     let data = json.parseFile(jsonFile.string)
+
+    let output = data["outputFile"].getStr
+    createDir output.parentDir
+    let outputCurrent = $conf.absOutFile
+    if output != outputCurrent:
+      # previously, any specified output file would be silently ignored;
+      # simply copying won't work in some cases, for example with `extraCmds`,
+      # so we just make it an error, user should use same command for jsonscript
+      # as was used with --compileOnly.
+      globalError(conf, gCmdLineInfo, "jsonscript command outputFile '$1' must match '$2' which was specified during --compileOnly, see \"outputFile\" entry in '$3' " % [outputCurrent, output, jsonFile.string])
+
     let toCompile = data["compile"]
     doAssert toCompile.kind == JArray
     var cmds: TStringSeq

compiler/main.nim

@@ -66,8 +66,7 @@ when not defined(leanCompiler):
     compileProject(graph)
     finishDoc2Pass(graph.config.projectName)
 
-proc commandCompileToC(graph: ModuleGraph) =
-  let conf = graph.config
+proc setOutFile(conf: ConfigRef) =
   if conf.outFile.isEmpty:
     let base = conf.projectName
     let targetName = if optGenDynLib in conf.globalOptions:
@@ -76,6 +75,9 @@ proc commandCompileToC(graph: ModuleGraph) =
       base & platform.OS[conf.target.targetOS].exeExt
     conf.outFile = RelativeFile targetName
 
+proc commandCompileToC(graph: ModuleGraph) =
+  let conf = graph.config
+  setOutFile(conf)
   extccomp.initVars(conf)
   semanticPasses(graph)
   registerPass(graph, cgenPass)
@@ -370,6 +372,7 @@ proc mainCommand*(graph: ModuleGraph) =
     conf.cmd = cmdDump
   of "jsonscript":
     conf.cmd = cmdJsonScript
+    setOutFile(graph.config)
     commandJsonScript(graph)
   elif commandAlreadyProcessed: discard # already handled
   else:
@@ -386,7 +389,15 @@ proc mainCommand*(graph: ModuleGraph) =
                 else: "Debug"
     let sec = formatFloat(epochTime() - conf.lastCmdTime, ffDecimal, 3)
     let project = if optListFullPaths in conf.globalOptions: $conf.projectFull else: $conf.projectName
-    var output = $conf.absOutFile
+
+    var output: string
+    if optCompileOnly in conf.globalOptions and conf.cmd != cmdJsonScript:
+      output = $conf.jsonBuildFile
+    elif conf.outFile.isEmpty and conf.cmd notin {cmdJsonScript, cmdCompileToBackend, cmdDoc}:
+      # for some cmd we expect a valid absOutFile
+      output = "unknownOutput"
+    else:
+      output = $conf.absOutFile
     if optListFullPaths notin conf.globalOptions: output = output.AbsoluteFile.extractFilename
     rawMessage(conf, hintSuccessX, [
       "loc", loc,

compiler/modules.nim

@@ -17,22 +17,29 @@ import
 proc resetSystemArtifacts*(g: ModuleGraph) =
   magicsys.resetSysTypes(g)
 
-proc partialInitModule(result: PSym; graph: ModuleGraph; fileIdx: FileIndex; filename: AbsoluteFile) =
+template getModuleIdent(graph: ModuleGraph, filename: AbsoluteFile): PIdent =
+  getIdent(graph.cache, splitFile(filename).name)
+
+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)
   var packSym = graph.packageSyms.strTableGet(pack)
   if packSym == nil:
-    packSym = newSym(skPackage, getIdent(graph.cache, pck2), nil, result.info)
+    packSym = newSym(skPackage, getIdent(graph.cache, pck2), nil, info)
     initStrTable(packSym.tab)
     graph.packageSyms.strTableAdd(packSym)
   else:
-    let existing = strTableGet(packSym.tab, result.name)
-    if existing != nil and existing.info.fileIndex != result.info.fileIndex:
+    let existing = strTableGet(packSym.tab, name)
+    if existing != nil and existing.info.fileIndex != info.fileIndex:
       when false:
         # we used to produce an error:
-        localError(graph.config, result.info,
+        localError(graph.config, info,
           "module names need to be unique per Nimble package; module clashes with " &
             toFullPath(graph.config, existing.info.fileIndex))
       else:
@@ -40,10 +47,13 @@ proc partialInitModule(result: PSym; graph: ModuleGraph; fileIdx: FileIndex; fil
         # to resolve the conflicts:
         let pck3 = fakePackageName(graph.config, filename)
         # this makes the new `packSym`'s owner be the original `packSym`
-        packSym = newSym(skPackage, getIdent(graph.cache, pck3), packSym, result.info)
+        packSym = newSym(skPackage, getIdent(graph.cache, pck3), packSym, info)
         initStrTable(packSym.tab)
         graph.packageSyms.strTableAdd(packSym)
+  result = packSym
 
+proc partialInitModule(result: PSym; graph: ModuleGraph; fileIdx: FileIndex; filename: AbsoluteFile) =
+  let packSym = getPackage(graph, fileIdx)
   result.owner = packSym
   result.position = int fileIdx
 
@@ -60,13 +70,15 @@ proc newModule(graph: ModuleGraph; fileIdx: FileIndex): PSym =
   # We cannot call ``newSym`` here, because we have to circumvent the ID
   # mechanism, which we do in order to assign each module a persistent ID.
   result = PSym(kind: skModule, id: -1, # for better error checking
-                name: getIdent(graph.cache, splitFile(filename).name),
+                name: getModuleIdent(graph, filename),
                 info: newLineInfo(fileIdx, 1, 1))
   if not isNimIdentifier(result.name.s):
     rawMessage(graph.config, errGenerated, "invalid module name: " & result.name.s)
   partialInitModule(result, graph, fileIdx, filename)
 
 proc compileModule*(graph: ModuleGraph; fileIdx: FileIndex; flags: TSymFlags): PSym =
+  var flags = flags
+  if fileIdx == graph.config.projectMainIdx2: flags.incl sfMainModule
   result = graph.getModule(fileIdx)
   if result == nil:
     let filename = AbsoluteFile toFullPath(graph.config, fileIdx)
@@ -104,7 +116,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.owner.id == graph.config.mainPackageId or isDefined(graph.config, "booting"): graph.config.mainPackageNotes
+    if s.getnimblePkgId == graph.config.mainPackageId or isDefined(graph.config, "booting"): graph.config.mainPackageNotes
     else: graph.config.foreignPackageNotes
 
 proc includeModule*(graph: ModuleGraph; s: PSym, fileIdx: FileIndex): PNode =
@@ -135,7 +147,12 @@ proc compileProject*(graph: ModuleGraph; projectFileIdx = InvalidFileIdx) =
   wantMainModule(conf)
   let systemFileIdx = fileInfoIdx(conf, conf.libpath / RelativeFile"system.nim")
   let projectFile = if projectFileIdx == InvalidFileIdx: conf.projectMainIdx else: projectFileIdx
+  conf.projectMainIdx2 = projectFile
+
+  let packSym = getPackage(graph, projectFile)
+  graph.config.mainPackageId = packSym.getnimblePkgId
   graph.importStack.add projectFile
+
   if projectFile == systemFileIdx:
     discard graph.compileModule(projectFile, {sfMainModule, sfSystemModule})
   else:

compiler/msgs.nim

@@ -117,6 +117,8 @@ proc newLineInfo*(fileInfoIdx: FileIndex, line, col: int): TLineInfo =
 proc newLineInfo*(conf: ConfigRef; filename: AbsoluteFile, line, col: int): TLineInfo {.inline.} =
   result = newLineInfo(fileInfoIdx(conf, filename), line, col)
 
+const gCmdLineInfo* = newLineInfo(commandLineIdx, 1, 1)
+
 proc concat(strings: openArray[string]): string =
   var totalLen = 0
   for s in strings: totalLen += s.len

compiler/options.nim

@@ -273,6 +273,7 @@ type
     lazyPaths*: seq[AbsoluteDir]
     outFile*: RelativeFile
     outDir*: AbsoluteDir
+    jsonBuildFile*: AbsoluteFile
     prefixDir*, libpath*, nimcacheDir*: AbsoluteDir
     dllOverrides, moduleOverrides*, cfileSpecificOptions*: StringTableRef
     projectName*: string # holds a name like 'nim'
@@ -281,6 +282,7 @@ type
     projectIsStdin*: bool # whether we're compiling from stdin
     lastMsgWasDot*: bool # the last compiler message was a single '.'
     projectMainIdx*: FileIndex # the canonical path id of the main module
+    projectMainIdx2*: FileIndex # consider merging with projectMainIdx
     command*: string # the main command (e.g. cc, check, scan, etc)
     commandArgs*: seq[string] # any arguments after the main command
     commandLine*: string
@@ -336,7 +338,10 @@ proc setNote*(conf: ConfigRef, note: TNoteKind, enabled = true) =
     if enabled: incl(conf.notes, note) else: excl(conf.notes, note)
 
 proc hasHint*(conf: ConfigRef, note: TNoteKind): bool =
-  optHints in conf.options and note in conf.notes
+  if optHints notin conf.options: false
+  elif note in {hintConf}: # could add here other special notes like hintSource
+    note in conf.mainPackageNotes
+  else: note in conf.notes
 
 proc hasWarn*(conf: ConfigRef, note: TNoteKind): bool =
   optWarns in conf.options and note in conf.notes
@@ -556,11 +561,8 @@ proc getOutFile*(conf: ConfigRef; filename: RelativeFile, ext: string): Absolute
   result = conf.outDir / changeFileExt(filename, ext)
 
 proc absOutFile*(conf: ConfigRef): AbsoluteFile =
-  if false:
-    doAssert not conf.outDir.isEmpty
-    doAssert not conf.outFile.isEmpty
-    # xxx: fix this pre-existing bug causing `SuccessX` error messages to lie
-    # for `jsonscript`
+  doAssert not conf.outDir.isEmpty
+  doAssert not conf.outFile.isEmpty
   result = conf.outDir / conf.outFile
   when defined(posix):
     if dirExists(result.string): result.string.add ".out"

compiler/passes.nim

@@ -113,10 +113,8 @@ const
     nkExportStmt, nkExportExceptStmt, nkFromStmt, nkImportStmt, nkImportExceptStmt}
 
 proc prepareConfigNotes(graph: ModuleGraph; module: PSym) =
-  if sfMainModule in module.flags:
-    graph.config.mainPackageId = module.owner.id
   # don't be verbose unless the module belongs to the main package:
-  if module.owner.id == graph.config.mainPackageId:
+  if module.getnimblePkgId == graph.config.mainPackageId:
     graph.config.notes = graph.config.mainPackageNotes
   else:
     if graph.config.mainPackageNotes == {}: graph.config.mainPackageNotes = graph.config.notes