Skip to content

Commit 36124cb

Browse files
committed
fix(scan): an entry source outside the globs is read by the scanner
A target's entry source that no `sources` glob matched (a discovered test, a `main` outside the globs) had its imports read from line-leading `import` alone, in make_plan and again in the standard library check before planning. An import inside a comment or a raw string was therefore planned as one. Validating the lsp-mcpp repository's own build database against S1 found it: its scanner test was planned as importing three modules no source provides. scan_entry_file reads such a file with scan_file. The scanner's refusals (an import inside `#if`, a header unit, an extension without a role) were never applied on this path and are not applied now: a file it refuses yields the line-leading imports of its code, comments and raw strings removed, with the declaration form Unknown. Both readers now call it, so they cannot disagree about `import std`. Refs #636
1 parent 39053b9 commit 36124cb

4 files changed

Lines changed: 101 additions & 62 deletions

File tree

‎src/build/plan.cppm‎

Lines changed: 10 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -1919,28 +1919,16 @@ make_plan(const mcpp::manifest::Manifest& manifest,
19191919
mcpp::modgraph::normalize_include_flags(projectRoot, main_cu.packageCflags);
19201920
mcpp::modgraph::normalize_include_flags(projectRoot, main_cu.packageCxxflags);
19211921

1922-
// We didn't scan main.cpp earlier (it's not in scanner output unless globbed in).
1923-
// Best-effort: scan its imports here.
1924-
std::ifstream is(*lu.entryMain);
1925-
std::string line;
1926-
while (std::getline(is, line)) {
1927-
auto trim = [](std::string s) {
1928-
while (!s.empty() && std::isspace(static_cast<unsigned char>(s.front()))) s.erase(0, 1);
1929-
while (!s.empty() && std::isspace(static_cast<unsigned char>(s.back()))) s.pop_back();
1930-
return s;
1931-
};
1932-
line = trim(line);
1933-
if (line.starts_with("import ")) {
1934-
std::string name;
1935-
std::size_t i = 7;
1936-
while (i < line.size() && (std::isalnum(static_cast<unsigned char>(line[i]))
1937-
|| line[i] == '_' || line[i] == '.')) {
1938-
name.push_back(line[i]);
1939-
++i;
1940-
}
1941-
if (!name.empty()) main_cu.imports.push_back(name);
1942-
}
1943-
}
1922+
// The entry is in the package scan only when a `sources` glob
1923+
// matched it; otherwise it is scanned here. The unit is built as one
1924+
// that provides nothing, so a declaration the scanner reads as
1925+
// providing a module is recorded as undecided rather than as a
1926+
// role the plan does not build.
1927+
const auto entry = mcpp::modgraph::scan_entry_file(
1928+
*lu.entryMain, main_cu.packageName, rootExtTable);
1929+
for (auto const& req : entry.requires_) main_cu.imports.push_back(req.logicalName);
1930+
main_cu.declaration = entry.provides ? mcpp::modgraph::ModuleDeclaration::Unknown
1931+
: entry.declaration;
19441932

19451933
// mcpp#240: the entry main may ALSO have been scanned (globbed into
19461934
// [modules].sources — the near-universal `src/**/*.cpp`). When it

‎src/build/prepare.cppm‎

Lines changed: 9 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -648,43 +648,6 @@ bool is_std_module(std::string_view name) {
648648
return name == "std" || name == "std.compat";
649649
}
650650

651-
std::string trim_copy(std::string s) {
652-
while (!s.empty() && std::isspace(static_cast<unsigned char>(s.front())))
653-
s.erase(0, 1);
654-
while (!s.empty() && std::isspace(static_cast<unsigned char>(s.back())))
655-
s.pop_back();
656-
return s;
657-
}
658-
659-
bool source_file_imports_std(const std::filesystem::path& path) {
660-
std::ifstream is(path);
661-
if (!is) return false;
662-
663-
std::string line;
664-
while (std::getline(is, line)) {
665-
line = trim_copy(std::move(line));
666-
std::size_t i = std::string::npos;
667-
if (line.starts_with("import ")) {
668-
i = 7;
669-
} else if (line.starts_with("export import ")) {
670-
i = 14;
671-
}
672-
if (i == std::string::npos) continue;
673-
while (i < line.size() && std::isspace(static_cast<unsigned char>(line[i])))
674-
++i;
675-
676-
std::string name;
677-
while (i < line.size()
678-
&& (std::isalnum(static_cast<unsigned char>(line[i]))
679-
|| line[i] == '_' || line[i] == '.' || line[i] == ':')) {
680-
name.push_back(line[i]);
681-
++i;
682-
}
683-
if (is_std_module(name)) return true;
684-
}
685-
return false;
686-
}
687-
688651
bool graph_or_targets_import_std(const mcpp::modgraph::Graph& graph,
689652
const mcpp::manifest::Manifest& manifest,
690653
const std::filesystem::path& projectRoot) {
@@ -696,10 +659,16 @@ bool graph_or_targets_import_std(const mcpp::modgraph::Graph& graph,
696659
}
697660

698661
// Some target entry files can be added to the plan after the package scan.
699-
// Check them here so std BMI setup matches what make_plan will compile.
662+
// Check them here so std BMI setup matches what make_plan will compile: they
663+
// are read by the same scan_entry_file make_plan reads them with.
664+
const auto extTable = mcpp::extension_table_for(manifest.buildConfig.moduleExtensions,
665+
manifest.buildConfig.deviceExtensions);
700666
for (auto& t : manifest.targets) {
701-
if (!t.main.empty() && source_file_imports_std(projectRoot / t.main))
702-
return true;
667+
if (t.main.empty()) continue;
668+
const auto entry = mcpp::modgraph::scan_entry_file(projectRoot / t.main,
669+
manifest.package.name, extTable);
670+
for (auto const& req : entry.requires_)
671+
if (is_std_module(req.logicalName)) return true;
703672
}
704673
return false;
705674
}

‎src/modgraph/scanner.cppm‎

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -63,6 +63,16 @@ std::expected<SourceUnit, ScanError> scan_file(const std::filesystem::path& file
6363
const std::string& packageName,
6464
const mcpp::ExtensionTable& extTable);
6565

66+
// Scan the entry source of a target that no `sources` glob matched: a
67+
// discovered test, or a `main` outside the globs. The unit is scan_file's when
68+
// scan_file accepts the file. Its refusals (an import inside `#if`, a header
69+
// unit, an extension without a role) were never applied to such a file and are
70+
// not applied here: a file it refuses yields the line-leading imports of its
71+
// code, with comments and raw strings removed, and the declaration form Unknown.
72+
SourceUnit scan_entry_file(const std::filesystem::path& file,
73+
const std::string& packageName,
74+
const mcpp::ExtensionTable& extTable);
75+
6676
// Scan the entire package: collects all sources via manifest globs and returns a Graph.
6777
struct ScanResult {
6878
Graph graph;
@@ -1005,6 +1015,34 @@ std::expected<SourceUnit, ScanError> scan_file(const std::filesystem::path& file
10051015
return u;
10061016
}
10071017

1018+
SourceUnit scan_entry_file(const std::filesystem::path& file,
1019+
const std::string& packageName,
1020+
const mcpp::ExtensionTable& extTable)
1021+
{
1022+
if (auto scanned = scan_file(file, packageName, extTable)) return std::move(*scanned);
1023+
1024+
SourceUnit u;
1025+
u.path = file;
1026+
u.packageName = packageName;
1027+
u.kind = mcpp::classify(file, extTable);
1028+
u.declaration = ModuleDeclaration::Unknown;
1029+
std::ifstream is(file);
1030+
bool in_raw = false, in_block = false;
1031+
std::string raw_close, line;
1032+
while (std::getline(is, line)) {
1033+
const std::string code = strip_noncode(line, in_block, in_raw, raw_close);
1034+
std::string_view r = trim(code);
1035+
if (r.starts_with("export ") || r.starts_with("export\t")) r = trim(r.substr(6));
1036+
if (!r.starts_with("import ") && !r.starts_with("import\t")) continue;
1037+
r = trim(r.substr(6));
1038+
std::string name;
1039+
for (std::size_t i = 0; i < r.size() && is_module_name_char(r[i]); ++i)
1040+
name.push_back(r[i]);
1041+
if (!name.empty() && name.front() != ':') u.requires_.push_back(ModuleId{name});
1042+
}
1043+
return u;
1044+
}
1045+
10081046
namespace {
10091047

10101048
std::vector<std::filesystem::path>

‎tests/unit/test_modgraph.cpp‎

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -128,6 +128,50 @@ TEST(Scanner, DeclarationFormIsRecordedAsRead) {
128128
std::filesystem::remove_all(dir);
129129
}
130130

131+
// ─── an entry source no `sources` glob matched ─────────────────────────────
132+
//
133+
// A discovered test, or a `main` outside the globs, is not in the package scan.
134+
// Its imports were read from line-leading `import` alone, so an import inside a
135+
// comment or a raw string was recorded as one: a scanner test in lsp-mcpp was
136+
// planned as importing three modules no source provides.
137+
TEST(Scanner, AnEntrySourceIsReadByTheScanner) {
138+
auto dir = make_tempdir("scan-entry");
139+
write(dir / "t.cpp",
140+
"import std;\n"
141+
"/* a comment\n"
142+
" import in.comment; */\n"
143+
"auto s = R\"x(\n"
144+
"import in.raw;\n"
145+
")x\";\n"
146+
"import real;\n"
147+
"int main() {}\n");
148+
auto u = scan_entry_file(dir / "t.cpp", "pkg", mcpp::builtin_extension_table());
149+
std::vector<std::string> names;
150+
for (auto const& r : u.requires_) names.push_back(r.logicalName);
151+
EXPECT_EQ(names, (std::vector<std::string>{"std", "real"}));
152+
EXPECT_EQ(static_cast<int>(u.declaration), static_cast<int>(ModuleDeclaration::None));
153+
std::filesystem::remove_all(dir);
154+
}
155+
156+
// The scanner's refusals were never applied to such a file, and are not now: a
157+
// file it refuses keeps its line-leading imports, and its form is not decided.
158+
TEST(Scanner, AnEntrySourceTheScannerRefusesKeepsItsImports) {
159+
auto dir = make_tempdir("scan-entry-refused");
160+
write(dir / "t.cpp",
161+
"#ifdef WITH_EXTRA\n"
162+
"import extra;\n"
163+
"#endif\n"
164+
"export import std;\n"
165+
"int main() {}\n");
166+
ASSERT_FALSE(scan_file(dir / "t.cpp", "pkg", mcpp::builtin_extension_table()).has_value());
167+
auto u = scan_entry_file(dir / "t.cpp", "pkg", mcpp::builtin_extension_table());
168+
std::vector<std::string> names;
169+
for (auto const& r : u.requires_) names.push_back(r.logicalName);
170+
EXPECT_EQ(names, (std::vector<std::string>{"extra", "std"}));
171+
EXPECT_EQ(static_cast<int>(u.declaration), static_cast<int>(ModuleDeclaration::Unknown));
172+
std::filesystem::remove_all(dir);
173+
}
174+
131175
TEST(Scanner, PlainImplementationUnitStillRequiresItsInterface) {
132176
auto dir = make_tempdir("scan-implunit");
133177
std::filesystem::create_directories(dir / "src");

0 commit comments

Comments
 (0)