From 074f276c8a753bbb85788777b7c58a074f41329f Mon Sep 17 00:00:00 2001 From: Araq Date: Wed, 23 Nov 2016 23:23:31 +0100 Subject: [PATCH] disallow recursive module dependencies --- compiler/importer.nim | 20 +++++++++++++++++--- compiler/modulegraphs.nim | 4 ++++ compiler/modules.nim | 1 + compiler/msgs.nim | 5 ++--- compiler/nim.nim | 6 +++--- doc/manual/modules.txt | 32 ++------------------------------ tests/modules/trecinca.nim | 4 ++-- tests/modules/trecincb.nim | 2 +- tests/modules/trecmod.nim | 5 +++++ tests/modules/trecmod2.nim | 5 +++++ tests/modules/tselfimport.nim | 2 +- tests/system/tdeepcopy.nim | 0 tools/nimsuggest/nimsuggest.nim | 4 ++-- web/news/e029_version_0_16_0.rst | 2 ++ 14 files changed, 47 insertions(+), 45 deletions(-) mode change 100755 => 100644 tests/system/tdeepcopy.nim diff --git a/compiler/importer.nim b/compiler/importer.nim index ce365c4dca..46e4c159f8 100644 --- a/compiler/importer.nim +++ b/compiler/importer.nim @@ -162,12 +162,26 @@ proc importModuleAs(n: PNode, realModule: PSym): PSym = proc myImportModule(c: PContext, n: PNode): PSym = var f = checkModuleName(n) if f != InvalidFileIDX: + let L = c.graph.importStack.len + let recursion = c.graph.importStack.find(f) + c.graph.importStack.add f + #echo "adding ", toFullPath(f), " at ", L+1 + if recursion >= 0: + var err = "" + for i in countup(recursion, L-1): + if i > 0: err.add "\n" + err.add toFullPath(c.graph.importStack[i]) & " imports " & + toFullPath(c.graph.importStack[i+1]) + localError(n.info, "recursive module dependency detected:\n" & err) result = importModuleAs(n, gImportModule(c.graph, c.module, f, c.cache)) + #echo "set back to ", L + c.graph.importStack.setLen(L) # we cannot perform this check reliably because of # test: modules/import_in_config) - if result.info.fileIndex == c.module.info.fileIndex and - result.info.fileIndex == n.info.fileIndex: - localError(n.info, errGenerated, "A module cannot import itself") + when false: + if result.info.fileIndex == c.module.info.fileIndex and + result.info.fileIndex == n.info.fileIndex: + localError(n.info, errGenerated, "A module cannot import itself") if sfDeprecated in result.flags: message(n.info, warnDeprecated, result.name.s) #suggestSym(n.info, result, false) diff --git a/compiler/modulegraphs.nim b/compiler/modulegraphs.nim index 9a3caa6632..38fd4f89fc 100644 --- a/compiler/modulegraphs.nim +++ b/compiler/modulegraphs.nim @@ -36,6 +36,8 @@ type invalidTransitiveClosure: bool inclToMod*: Table[int32, int32] # mapping of include file to the # first module that included it + importStack*: seq[int32] # The current import stack. Used for detecting recursive + # module dependencies. {.this: g.} @@ -44,12 +46,14 @@ proc newModuleGraph*(): ModuleGraph = initStrTable(result.packageSyms) result.deps = initIntSet() result.modules = @[] + result.importStack = @[] result.inclToMod = initTable[int32, int32]() proc resetAllModules*(g: ModuleGraph) = initStrTable(packageSyms) deps = initIntSet() modules = @[] + importStack = @[] inclToMod = initTable[int32, int32]() proc getModule*(g: ModuleGraph; fileIdx: int32): PSym = diff --git a/compiler/modules.nim b/compiler/modules.nim index 26ca2177b6..3451d85ecb 100644 --- a/compiler/modules.nim +++ b/compiler/modules.nim @@ -231,6 +231,7 @@ proc compileProject*(graph: ModuleGraph; cache: IdentCache; wantMainModule() let systemFileIdx = fileInfoIdx(options.libpath / "system.nim") let projectFile = if projectFileIdx < 0: gProjectMainIdx else: projectFileIdx + graph.importStack.add projectFile if projectFile == systemFileIdx: discard graph.compileModule(projectFile, cache, {sfMainModule, sfSystemModule}) else: diff --git a/compiler/msgs.nim b/compiler/msgs.nim index a44a1306c9..0f39eb4d3b 100644 --- a/compiler/msgs.nim +++ b/compiler/msgs.nim @@ -676,9 +676,8 @@ proc getInfoContext*(index: int): TLineInfo = if i >=% L: result = unknownLineInfo() else: result = msgContext[i] -proc toFilename*(fileIdx: int32): string = - if fileIdx < 0: result = "???" - else: result = fileInfos[fileIdx].projPath +template toFilename*(fileIdx: int32): string = + (if fileIdx < 0: "???" else: fileInfos[fileIdx].projPath) proc toFullPath*(fileIdx: int32): string = if fileIdx < 0: result = "???" diff --git a/compiler/nim.nim b/compiler/nim.nim index f8d6b607af..35afecf205 100644 --- a/compiler/nim.nim +++ b/compiler/nim.nim @@ -46,7 +46,7 @@ proc handleCmdLine(cache: IdentCache) = if gProjectName == "-": gProjectName = "stdinfile" gProjectFull = "stdinfile" - gProjectPath = getCurrentDir() + gProjectPath = canonicalizePath getCurrentDir() gProjectIsStdin = true elif gProjectName != "": try: @@ -54,10 +54,10 @@ proc handleCmdLine(cache: IdentCache) = except OSError: gProjectFull = gProjectName let p = splitFile(gProjectFull) - gProjectPath = p.dir + gProjectPath = canonicalizePath p.dir gProjectName = p.name else: - gProjectPath = getCurrentDir() + gProjectPath = canonicalizePath getCurrentDir() loadConfigs(DefaultConfig) # load all config files let scriptFile = gProjectFull.changeFileExt("nims") if fileExists(scriptFile): diff --git a/doc/manual/modules.txt b/doc/manual/modules.txt index 9cb6a11af8..8a9f5ff655 100644 --- a/doc/manual/modules.txt +++ b/doc/manual/modules.txt @@ -9,36 +9,8 @@ subtle. Only top-level symbols that are marked with an asterisk (``*``) are exported. A valid module name can only be a valid Nim identifier (and thus its filename is ``identifier.nim``). -The algorithm for compiling modules is: - -- compile the whole module as usual, following import statements recursively - -- if there is a cycle only import the already parsed symbols (that are - exported); if an unknown identifier occurs then abort - -This is best illustrated by an example: - -.. code-block:: nim - # Module A - type - T1* = int # Module A exports the type ``T1`` - import B # the compiler starts parsing B - - proc main() = - var i = p(3) # works because B has been parsed completely here - - main() - - -.. code-block:: nim - # Module B - import A # A is not parsed here! Only the already known symbols - # of A are imported. - - proc p*(x: A.T1): A.T1 = - # this works because the compiler has already - # added T1 to A's interface symbol table - result = x + 1 +Recursive module dependencies are not allowed. This restriction might be mitigated +or removed in later versions of the language. Import statement diff --git a/tests/modules/trecinca.nim b/tests/modules/trecinca.nim index 14a91ba5c3..7a74d7a465 100644 --- a/tests/modules/trecinca.nim +++ b/tests/modules/trecinca.nim @@ -1,7 +1,7 @@ discard """ - file: "tests/reject/trecincb.nim" + file: "trecincb.nim" line: 9 - errormsg: "recursive dependency: 'tests/modules/trecincb.nim'" + errormsg: "recursive dependency: 'trecincb.nim'" """ # Test recursive includes diff --git a/tests/modules/trecincb.nim b/tests/modules/trecincb.nim index 299a242e1c..1d3eb55035 100644 --- a/tests/modules/trecincb.nim +++ b/tests/modules/trecincb.nim @@ -1,7 +1,7 @@ discard """ file: "trecincb.nim" line: 9 - errormsg: "recursive dependency: 'tests/modules/trecincb.nim'" + errormsg: "recursive dependency: 'trecincb.nim'" """ # Test recursive includes diff --git a/tests/modules/trecmod.nim b/tests/modules/trecmod.nim index d567e293b3..c670bec558 100644 --- a/tests/modules/trecmod.nim +++ b/tests/modules/trecmod.nim @@ -1,2 +1,7 @@ +discard """ + file: "mrecmod.nim" + line: 1 + errormsg: "recursive module dependency detected" +""" # recursive module import mrecmod diff --git a/tests/modules/trecmod2.nim b/tests/modules/trecmod2.nim index 85fe2215fd..aa88f5e91c 100644 --- a/tests/modules/trecmod2.nim +++ b/tests/modules/trecmod2.nim @@ -1,3 +1,8 @@ +discard """ + file: "mrecmod2.nim" + line: 2 + errormsg: "recursive module dependency detected" +""" type T1* = int # Module A exports the type ``T1`` diff --git a/tests/modules/tselfimport.nim b/tests/modules/tselfimport.nim index ddb3a5b093..b9109deaec 100644 --- a/tests/modules/tselfimport.nim +++ b/tests/modules/tselfimport.nim @@ -1,7 +1,7 @@ discard """ file: "tselfimport.nim" line: 7 - errormsg: "A module cannot import itself" + errormsg: "recursive module dependency detected" """ import strutils as su # guard against regression import tselfimport #ERROR diff --git a/tests/system/tdeepcopy.nim b/tests/system/tdeepcopy.nim old mode 100755 new mode 100644 diff --git a/tools/nimsuggest/nimsuggest.nim b/tools/nimsuggest/nimsuggest.nim index 822ef7224a..b5e7b282f5 100644 --- a/tools/nimsuggest/nimsuggest.nim +++ b/tools/nimsuggest/nimsuggest.nim @@ -431,10 +431,10 @@ proc handleCmdLine(cache: IdentCache) = except OSError: gProjectFull = gProjectName var p = splitFile(gProjectFull) - gProjectPath = p.dir + gProjectPath = canonicalizePath p.dir gProjectName = p.name else: - gProjectPath = getCurrentDir() + gProjectPath = canonicalizePath getCurrentDir() # Find Nim's prefix dir. let binaryPath = findExe("nim") diff --git a/web/news/e029_version_0_16_0.rst b/web/news/e029_version_0_16_0.rst index 94c9757a73..42fdfc0e4d 100644 --- a/web/news/e029_version_0_16_0.rst +++ b/web/news/e029_version_0_16_0.rst @@ -29,6 +29,8 @@ Changes affecting backwards compatibility - ``TimeInfo.tzname`` has been removed from ``times`` module because it was broken. Because of this, the option ``"ZZZ"`` will no longer work in format strings for formatting and parsing. +- Recursive module dependencies are now completely disallowed. + Library Additions -----------------