Skip to content

Commit

Permalink
Fix error with certain include module patterns (#25888)
Browse files Browse the repository at this point in the history
Resolves #25569

This PR fixes a problem with using `import super.something` from within
a submodule that was stored in a different file using `include module`.

Reviewed by @DanilaFe - thanks!

- [x] full comm=none testing
  • Loading branch information
mppf authored Sep 9, 2024
2 parents 3284b94 + ae0e02b commit 08f4df2
Show file tree
Hide file tree
Showing 7 changed files with 48 additions and 3 deletions.
6 changes: 5 additions & 1 deletion frontend/include/chpl/parsing/parsing-queries.h
Original file line number Diff line number Diff line change
Expand Up @@ -107,9 +107,13 @@ introspectParsedFiles(Context* context);
/**
Like parseFileToBuilderResult but parses whatever file contained 'id'.
Useful for projection queries.
If setParentSymbolPath is not nullptr, sets it to the parentSymbolPath used
when creating the BuilderResult.
*/
const uast::BuilderResult*
parseFileContainingIdToBuilderResult(Context* context, ID id);
parseFileContainingIdToBuilderResult(Context* context, ID id,
UniqueString* setParentSymbolPath=nullptr);

/**
Fetch the BuilderResult storing compiler-generated uAST based on the given
Expand Down
22 changes: 20 additions & 2 deletions frontend/lib/parsing/parsing-queries.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -203,7 +203,9 @@ compilerGeneratedBuilderQuery(Context* context, UniqueString symbolPath) {

// parses whatever file exists that contains the passed ID and returns it
const BuilderResult*
parseFileContainingIdToBuilderResult(Context* context, ID id) {
parseFileContainingIdToBuilderResult(Context* context,
ID id,
UniqueString* setParentSymbolPath) {
if (id.isFabricatedId() &&
id.fabricatedIdKind() == ID::FabricatedIdKind::Generated) {
// Find the generated module's symbol path
Expand All @@ -219,6 +221,7 @@ parseFileContainingIdToBuilderResult(Context* context, ID id) {

const BuilderResult& br = getCompilerGeneratedBuilder(context, symbolPath);
assert(br.numTopLevelExpressions() != 0);
if (setParentSymbolPath) *setParentSymbolPath = UniqueString();
return &br;
} else {
UniqueString path;
Expand All @@ -227,6 +230,7 @@ parseFileContainingIdToBuilderResult(Context* context, ID id) {
if (found) {
const BuilderResult& p = parseFileToBuilderResult(context, path,
parentSymbolPath);
if (setParentSymbolPath) *setParentSymbolPath = parentSymbolPath;
return &p;
}

Expand Down Expand Up @@ -1425,11 +1429,25 @@ const ID& idToParentId(Context* context, ID id) {
// set this query as an alternative to computing maps
// in Builder::Result and then redundantly setting them here?

// Performance: Could this query use id.parentSymbolId in many cases?

ID result;

const BuilderResult* r = parseFileContainingIdToBuilderResult(context, id);
UniqueString parentSymbolPath;
const BuilderResult* r =
parseFileContainingIdToBuilderResult(context, id, &parentSymbolPath);

if (r != nullptr) {
result = r->idToParentId(id);
// For a submodule in a separate file, the BuilderResult's idToParentId
// will return an empty ID for the submodule.
// Detect that and return the parent module in that case.
if (result.isEmpty() && !parentSymbolPath.isEmpty()) {
ID parentSymbolId = id.parentSymbolId(context);
CHPL_ASSERT(!parentSymbolId.isEmpty());
CHPL_ASSERT(parentSymbolId.symbolPath() == parentSymbolPath);
result = parentSymbolId;
}
}

return QUERY_END(result);
Expand Down
13 changes: 13 additions & 0 deletions test/visibility/submodule-in-file/Recursive.chpl
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
// This (and SubModule) were created as a reproducer for issue #25569

module Recursive {
config param EXTRA_CHECKS = 42;

include public module SubModule;

use this.SubModule;

writeln("in Recursive, EXTRA_CHECKS=", EXTRA_CHECKS);

foo();
}
Empty file.
5 changes: 5 additions & 0 deletions test/visibility/submodule-in-file/Recursive/SubModule.chpl
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
module SubModule {
import super.EXTRA_CHECKS;

proc foo() { writeln("in SubModule.foo, EXTRA_CHECKS=", EXTRA_CHECKS); }
}
3 changes: 3 additions & 0 deletions test/visibility/submodule-in-file/test-Recursive.chpl
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
use Recursive;

proc main() { }
2 changes: 2 additions & 0 deletions test/visibility/submodule-in-file/test-Recursive.good
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
in Recursive, EXTRA_CHECKS=42
in SubModule.foo, EXTRA_CHECKS=42

0 comments on commit 08f4df2

Please sign in to comment.