Skip to content

Commit

Permalink
Fix 22730 - Promote imported files (-i) to root module before parsing...
Browse files Browse the repository at this point in the history
... the declarations.

DMD PR 13224 changed the parser s.t. unittests from non-root modules
are skipped, i.e. not even parsed. The new behaviour didn't work as
expected when combined with `-i` because it promoted imported files to
root modules *after* the parser processed the entire file.

This commit moves the existing checks s.t. they are applied immediatly
after the module declaration was read by the parser.
  • Loading branch information
MoonlightSentinel authored and thewilsonator committed Feb 16, 2022
1 parent c1c6e2c commit 1c31d85
Show file tree
Hide file tree
Showing 5 changed files with 88 additions and 20 deletions.
49 changes: 29 additions & 20 deletions src/dmd/dmodule.d
Original file line number Diff line number Diff line change
Expand Up @@ -526,15 +526,6 @@ extern (C++) final class Module : Package
}
m = m.parse();

// Call onImport here because if the module is going to be compiled then we
// need to determine it early because it affects semantic analysis. This is
// being done after parsing the module so the full module name can be taken
// from whatever was declared in the file.
if (!m.isRoot() && Compiler.onImport(m))
{
m.importedFrom = m;
assert(m.isRoot());
}
return m;
}

Expand Down Expand Up @@ -962,6 +953,16 @@ extern (C++) final class Module : Package
isHdrFile = true;
}

/// Promote `this` to a root module if requested via `-i`
void checkCompiledImport()
{
if (!this.isRoot() && Compiler.onImport(this))
this.importedFrom = this;
}

DsymbolTable dst;
Package ppack = null;

/* If it has the extension ".c", it is a "C" file.
* If it has the extension ".i", it is a preprocessed "C" file.
*/
Expand All @@ -971,33 +972,41 @@ extern (C++) final class Module : Package

scope p = new CParser!AST(this, buf, cast(bool) docfile, target.c);
p.nextToken();
checkCompiledImport();
members = p.parseModule();
md = p.md;
assert(!p.md); // C doesn't have module declarations
numlines = p.scanloc.linnum;
}
else
{
scope p = new Parser!AST(this, buf, cast(bool) docfile);
p.nextToken();
members = p.parseModule();
p.parseModuleDeclaration();
md = p.md;

if (md)
{
/* A ModuleDeclaration, md, was provided.
* The ModuleDeclaration sets the packages this module appears in, and
* the name of this module.
*/
this.ident = md.id;
dst = Package.resolve(md.packages, &this.parent, &ppack);
}

// Done after parsing the module header because `module x.y.z` may override the file name
checkCompiledImport();

members = p.parseModuleContent();
numlines = p.scanloc.linnum;
}
srcBuffer.destroy();
srcBuffer = null;
/* The symbol table into which the module is to be inserted.
*/
DsymbolTable dst;

if (md)
{
/* A ModuleDeclaration, md, was provided.
* The ModuleDeclaration sets the packages this module appears in, and
* the name of this module.
*/
this.ident = md.id;
Package ppack = null;
dst = Package.resolve(md.packages, &this.parent, &ppack);

// Mark the package path as accessible from the current module
// https://issues.dlang.org/show_bug.cgi?id=21661
// Code taken from Import.addPackageAccess()
Expand Down
12 changes: 12 additions & 0 deletions test/compilable/imports/include_unittest/compiled_lib.d
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@

unittest
{
// This should trigger because this is a root module
pragma(msg, "Compiling compiled_lib.unittests");
}

void someFunction()
{
// This should trigger because this is a root module
pragma(msg, "Compiling compiled_lib.someFunction");
}
13 changes: 13 additions & 0 deletions test/compilable/imports/include_unittest/compiled_unittest_lib.d
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
module lib.with_.unittests;

void someFunction()
{
// This should trigger because this is a root module
pragma(msg, "Compiling lib.with_.unittests.someFunction");
}

unittest
{
// This should trigger because this is a root module
pragma(msg, "Compiling lib.with_.unittests.unittest");
}
15 changes: 15 additions & 0 deletions test/compilable/imports/include_unittest/skipped_unittest_lib.d
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
module lib.ignores.unittests;

pragma(msg, "Found module with skipped unittests");

unittest
{
// Shouldn't be parsed because we're in a non-root module
static assert(false, "Semantic on unittest in non-root module!");
}

void someFunction()
{
// This shouldn't be evaluated, no semantic for the body in non-root modules
static assert(false, "Semantic on function body in non-root module!");
}
19 changes: 19 additions & 0 deletions test/compilable/include_unittest.d
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
/+
https://issues.dlang.org/show_bug.cgi?id=22730
EXTRA_FILES: imports/include_unittest/compiled_lib.d import imports/include_unittest/skipped_unittest_lib.d import imports/include_unittest/compiled_unittest_lib.d
REQUIRED_ARGS: -i=compiled_lib -i=lib.with_.unittests -unittest
TEST_OUTPUT:
---
Found module with skipped unittests
Compiling compiled_lib.unittests
Compiling compiled_lib.someFunction
Compiling lib.with_.unittests.someFunction
Compiling lib.with_.unittests.unittest
---
+/

import imports.include_unittest.compiled_lib; // Matches the first -i pattern, no module decl.
import imports.include_unittest.skipped_unittest_lib; // Matches neither -i pattern
import imports.include_unittest.compiled_unittest_lib; // Matches the second -i pattern, has module decl.

0 comments on commit 1c31d85

Please sign in to comment.