https://github.com/yronglin updated https://github.com/llvm/llvm-project/pull/187858
>From 9f884d00dfb41e2bb04e0e29ca766d891fd790b6 Mon Sep 17 00:00:00 2001 From: yronglin <[email protected]> Date: Sun, 22 Mar 2026 12:12:12 +0800 Subject: [PATCH 1/3] [clangd] Handle C++20 annot_module_name token Signed-off-by: yronglin <[email protected]> --- .../clangd/test/non-existent.test | 82 +++++++++++++++++++ clang/lib/Tooling/Syntax/Tokens.cpp | 23 +++++- clang/unittests/Tooling/Syntax/TokensTest.cpp | 34 +++++++- 3 files changed, 134 insertions(+), 5 deletions(-) create mode 100644 clang-tools-extra/clangd/test/non-existent.test diff --git a/clang-tools-extra/clangd/test/non-existent.test b/clang-tools-extra/clangd/test/non-existent.test new file mode 100644 index 0000000000000..dd46ce25bc884 --- /dev/null +++ b/clang-tools-extra/clangd/test/non-existent.test @@ -0,0 +1,82 @@ +# +## reproduce a crash in module processing - seems having a non-existent module imported +## as the last statement in a file causes clangd to crash +## modified from modules.test +# +# Windows have different escaping modes. +# FIXME: We should add one for windows. +# UNSUPPORTED: system-windows +# +# RUN: rm -fr %t +# RUN: mkdir -p %t +# RUN: split-file %s %t +# +# RUN: sed -e "s|DIR|%/t|g" %t/compile_commands.json.tmpl > %t/compile_commands.json.tmp +# RUN: sed -e "s|CLANG_CC|%clang|g" %t/compile_commands.json.tmp > %t/compile_commands.json +# RUN: sed -e "s|DIR|%/t|g" %t/definition.jsonrpc.tmpl > %t/definition.jsonrpc +# +# RUN: clangd -experimental-modules-support -lit-test < %t/definition.jsonrpc + +#--- A.cppm +module; +export module A; + +#--- Use.cpp +module; +export module Use; +import A; +import NonExistent; + +#--- compile_commands.json.tmpl +[ + { + "directory": "DIR", + "command": "CLANG_CC -fprebuilt-module-path=DIR -std=c++20 -o DIR/main.cpp.o DIR/Use.cpp -fmodule-file=A=DIR/A.pcm", + "file": "DIR/Use.cpp" + "output": "DIR/main.cpp.o" + }, + { + "directory": "DIR", + "command": "CLANG_CC -fprebuilt-module-path=DIR --std=c++20 DIR/A.cppm --precompile -o DIR/A.pcm", + "file": "DIR/A.cppm" + "output": "DIR/A.pcm" + } +] + +#--- definition.jsonrpc.tmpl +{ + "jsonrpc": "2.0", + "id": 0, + "method": "initialize", + "params": { + "processId": 123, + "rootPath": "clangd", + "capabilities": { + "textDocument": { + "completion": { + "completionItem": { + "snippetSupport": true + } + } + } + }, + "trace": "off" + } +} +--- +{ + "jsonrpc": "2.0", + "method": "textDocument/didOpen", + "params": { + "textDocument": { + "uri": "file://DIR/Use.cpp", + "languageId": "cpp", + "version": 1, + "text": "module;\nexport module Use;\nimport A;\nimport NonExistent;\n" + } + } +} +--- +{"jsonrpc":"2.0","id":2,"method":"shutdown"} +--- +{"jsonrpc":"2.0","method":"exit"} diff --git a/clang/lib/Tooling/Syntax/Tokens.cpp b/clang/lib/Tooling/Syntax/Tokens.cpp index 9ad8a149675d9..68443b4022916 100644 --- a/clang/lib/Tooling/Syntax/Tokens.cpp +++ b/clang/lib/Tooling/Syntax/Tokens.cpp @@ -681,8 +681,18 @@ class TokenCollector::CollectPPExpansions : public PPCallbacks { TokenCollector::TokenCollector(Preprocessor &PP) : PP(PP) { // Collect the expanded token stream during preprocessing. PP.setTokenWatcher([this](const clang::Token &T) { - if (T.isAnnotation()) + if (T.is(tok::annot_module_name)) { + auto &SM = this->PP.getSourceManager(); + StringRef Text = Lexer::getSourceText( + CharSourceRange::getTokenRange(T.getAnnotationRange()), SM, + this->PP.getLangOpts()); + Expanded.push_back( + syntax::Token(T.getLocation(), Text.size(), tok::annot_module_name)); + } else if (T.isAnnotation()) { return; + } else { + Expanded.push_back(syntax::Token(T)); + } DEBUG_WITH_TYPE("collect-tokens", llvm::dbgs() << "Token: " << syntax::Token(T).dumpForTests( @@ -690,7 +700,6 @@ TokenCollector::TokenCollector(Preprocessor &PP) : PP(PP) { << "\n" ); - Expanded.push_back(syntax::Token(T)); }); // And locations of macro calls, to properly recover boundaries of those in // case of empty expansions. @@ -901,6 +910,16 @@ class TokenCollector::Builder { TokenBuffer TokenCollector::consume() && { PP.setTokenWatcher(nullptr); Collector->disable(); + + /// If the parser hit an module load fatal error, the TokenCollector will not + /// receive an EOF token; we need to add an EOF token to the end of the token + /// sequence. + if (PP.hadModuleLoaderFatalFailure() && + (Expanded.empty() || Expanded.back().kind() != tok::eof)) { + auto &SM = PP.getSourceManager(); + Expanded.push_back( + syntax::Token(SM.getLocForEndOfFile(SM.getMainFileID()), 0, tok::eof)); + } return Builder(std::move(Expanded), std::move(Expansions), PP.getSourceManager(), PP.getLangOpts()) .build(); diff --git a/clang/unittests/Tooling/Syntax/TokensTest.cpp b/clang/unittests/Tooling/Syntax/TokensTest.cpp index 8af9308828c28..ae5001a2f5645 100644 --- a/clang/unittests/Tooling/Syntax/TokensTest.cpp +++ b/clang/unittests/Tooling/Syntax/TokensTest.cpp @@ -124,7 +124,7 @@ class TokenCollectorTest : public ::testing::Test { // Prepare to run a compiler. if (!Diags->getClient()) Diags->setClient(new IgnoringDiagConsumer); - std::vector<const char *> Args = {"tok-test", "-std=c++03", + std::vector<const char *> Args = {"tok-test", LangStandard.c_str(), "-fsyntax-only"}; Args.insert(Args.end(), ExtraArgs.begin(), ExtraArgs.end()); Args.push_back(FileName); @@ -144,8 +144,11 @@ class TokenCollectorTest : public ::testing::Test { this->Buffer = TokenBuffer(*SourceMgr); RecordTokens Recorder(this->Buffer); - ASSERT_TRUE(Compiler.ExecuteAction(Recorder)) - << "failed to run the frontend"; + if (AllowErrors) + Compiler.ExecuteAction(Recorder); + else + ASSERT_TRUE(Compiler.ExecuteAction(Recorder)) + << "failed to run the frontend"; } /// Record the tokens and return a test dump of the resulting buffer. @@ -253,6 +256,8 @@ class TokenCollectorTest : public ::testing::Test { } // Data fields. + std::string LangStandard = "-std=c++03"; + bool AllowErrors = false; DiagnosticOptions DiagOpts; llvm::IntrusiveRefCntPtr<DiagnosticsEngine> Diags = llvm::makeIntrusiveRefCnt<DiagnosticsEngine>(DiagnosticIDs::create(), @@ -1161,4 +1166,27 @@ TEST_F(TokenBufferTest, EofTokenOnBracketDepthLimit) { EXPECT_EQ(Buffer.expandedTokens().back().kind(), tok::eof); EXPECT_EQ(Buffer.expandedTokens().drop_back().back().kind(), tok::l_paren); } + +TEST_F(TokenCollectorTest, CXX20ModuleImportModule) { + LangStandard = "-std=c++20"; + AllowErrors = true; + + recordTokens("import Non.Existent;\n"); + EXPECT_THAT(Buffer.expandedTokens(), + ElementsAre(Kind(tok::kw_import), Kind(tok::annot_module_name), + Kind(tok::semi), Kind(tok::eof))); +} + +TEST_F(TokenCollectorTest, CXX20ModuleImportPartition) { + LangStandard = "-std=c++20"; + AllowErrors = true; + + recordTokens("export module M.N;\nimport :Non.Existent;\n"); + EXPECT_THAT(Buffer.expandedTokens(), + ElementsAre(Kind(tok::kw_export), Kind(tok::kw_module), + Kind(tok::annot_module_name), Kind(tok::semi), + Kind(tok::kw_import), Kind(tok::colon), + Kind(tok::annot_module_name), Kind(tok::semi), + Kind(tok::eof))); +} } // namespace >From 0e96deef87095c72e288ad071476caaabeac6399 Mon Sep 17 00:00:00 2001 From: yronglin <[email protected]> Date: Tue, 24 Mar 2026 06:24:48 +0800 Subject: [PATCH 2/3] [clangd][ModulesBuilder] Don't discard successfully built modules when one fails Signed-off-by: yronglin <[email protected]> --- clang-tools-extra/clangd/ModulesBuilder.cpp | 9 +++++++-- clang-tools-extra/clangd/test/non-existent.test | 4 ++-- clang/lib/Tooling/Syntax/Tokens.cpp | 9 --------- 3 files changed, 9 insertions(+), 13 deletions(-) diff --git a/clang-tools-extra/clangd/ModulesBuilder.cpp b/clang-tools-extra/clangd/ModulesBuilder.cpp index 706fd459e15ec..5e21b50df05bc 100644 --- a/clang-tools-extra/clangd/ModulesBuilder.cpp +++ b/clang-tools-extra/clangd/ModulesBuilder.cpp @@ -472,6 +472,9 @@ class ReusablePrerequisiteModules : public PrerequisiteModules { ~ReusablePrerequisiteModules() override = default; + bool hasFailures() const { return HasBuildFailure; } + void setHasFailures() { HasBuildFailure = true; } + void adjustHeaderSearchOptions(HeaderSearchOptions &Options) const override { // Appending all built module files. for (const auto &RequiredModule : RequiredModules) @@ -506,6 +509,7 @@ class ReusablePrerequisiteModules : public PrerequisiteModules { llvm::SmallVector<std::shared_ptr<const ModuleFile>, 8> RequiredModules; // A helper class to speedup the query if a module is built. llvm::StringSet<> BuiltModuleNames; + bool HasBuildFailure = false; }; bool IsModuleFileUpToDate(PathRef ModuleFilePath, @@ -696,6 +700,8 @@ copyModuleFileForRead(llvm::StringRef ModuleName, bool ReusablePrerequisiteModules::canReuse( const CompilerInvocation &CI, llvm::IntrusiveRefCntPtr<llvm::vfs::FileSystem> VFS) const { + if (hasFailures()) + return false; if (RequiredModules.empty()) return true; @@ -1246,12 +1252,11 @@ ModulesBuilder::buildPrerequisiteModulesFor(PathRef File, auto RequiredModules = std::make_unique<ReusablePrerequisiteModules>(); for (llvm::StringRef RequiredModuleName : RequiredModuleNames) { - // Return early if there is any error. if (llvm::Error Err = Impl->getOrBuildModuleFile( File, RequiredModuleName, TFS, CachedMDB, *RequiredModules.get())) { elog("Failed to build module {0}; due to {1}", RequiredModuleName, toString(std::move(Err))); - return std::make_unique<FailedPrerequisiteModules>(); + RequiredModules->setHasFailures(); } } diff --git a/clang-tools-extra/clangd/test/non-existent.test b/clang-tools-extra/clangd/test/non-existent.test index dd46ce25bc884..06c45d535241b 100644 --- a/clang-tools-extra/clangd/test/non-existent.test +++ b/clang-tools-extra/clangd/test/non-existent.test @@ -32,13 +32,13 @@ import NonExistent; { "directory": "DIR", "command": "CLANG_CC -fprebuilt-module-path=DIR -std=c++20 -o DIR/main.cpp.o DIR/Use.cpp -fmodule-file=A=DIR/A.pcm", - "file": "DIR/Use.cpp" + "file": "DIR/Use.cpp", "output": "DIR/main.cpp.o" }, { "directory": "DIR", "command": "CLANG_CC -fprebuilt-module-path=DIR --std=c++20 DIR/A.cppm --precompile -o DIR/A.pcm", - "file": "DIR/A.cppm" + "file": "DIR/A.cppm", "output": "DIR/A.pcm" } ] diff --git a/clang/lib/Tooling/Syntax/Tokens.cpp b/clang/lib/Tooling/Syntax/Tokens.cpp index 68443b4022916..9d9d779e931ca 100644 --- a/clang/lib/Tooling/Syntax/Tokens.cpp +++ b/clang/lib/Tooling/Syntax/Tokens.cpp @@ -911,15 +911,6 @@ TokenBuffer TokenCollector::consume() && { PP.setTokenWatcher(nullptr); Collector->disable(); - /// If the parser hit an module load fatal error, the TokenCollector will not - /// receive an EOF token; we need to add an EOF token to the end of the token - /// sequence. - if (PP.hadModuleLoaderFatalFailure() && - (Expanded.empty() || Expanded.back().kind() != tok::eof)) { - auto &SM = PP.getSourceManager(); - Expanded.push_back( - syntax::Token(SM.getLocForEndOfFile(SM.getMainFileID()), 0, tok::eof)); - } return Builder(std::move(Expanded), std::move(Expansions), PP.getSourceManager(), PP.getLangOpts()) .build(); >From 01f35d98950f994bcd3cbe81572c2aee922bafe3 Mon Sep 17 00:00:00 2001 From: yronglin <[email protected]> Date: Sat, 16 May 2026 02:01:00 -0700 Subject: [PATCH 3/3] [clang] Don't cutoff parsing when load C++ named module failed Signed-off-by: yronglin <[email protected]> --- clang-tools-extra/clangd/ModulesBuilder.cpp | 9 ++------- clang/lib/Parse/Parser.cpp | 8 +++++++- clang/lib/Tooling/Syntax/Tokens.cpp | 1 - .../test/Modules/cxx20-fatal-module-loader-error.cpp | 12 ++++++++++++ 4 files changed, 21 insertions(+), 9 deletions(-) create mode 100644 clang/test/Modules/cxx20-fatal-module-loader-error.cpp diff --git a/clang-tools-extra/clangd/ModulesBuilder.cpp b/clang-tools-extra/clangd/ModulesBuilder.cpp index 5e21b50df05bc..706fd459e15ec 100644 --- a/clang-tools-extra/clangd/ModulesBuilder.cpp +++ b/clang-tools-extra/clangd/ModulesBuilder.cpp @@ -472,9 +472,6 @@ class ReusablePrerequisiteModules : public PrerequisiteModules { ~ReusablePrerequisiteModules() override = default; - bool hasFailures() const { return HasBuildFailure; } - void setHasFailures() { HasBuildFailure = true; } - void adjustHeaderSearchOptions(HeaderSearchOptions &Options) const override { // Appending all built module files. for (const auto &RequiredModule : RequiredModules) @@ -509,7 +506,6 @@ class ReusablePrerequisiteModules : public PrerequisiteModules { llvm::SmallVector<std::shared_ptr<const ModuleFile>, 8> RequiredModules; // A helper class to speedup the query if a module is built. llvm::StringSet<> BuiltModuleNames; - bool HasBuildFailure = false; }; bool IsModuleFileUpToDate(PathRef ModuleFilePath, @@ -700,8 +696,6 @@ copyModuleFileForRead(llvm::StringRef ModuleName, bool ReusablePrerequisiteModules::canReuse( const CompilerInvocation &CI, llvm::IntrusiveRefCntPtr<llvm::vfs::FileSystem> VFS) const { - if (hasFailures()) - return false; if (RequiredModules.empty()) return true; @@ -1252,11 +1246,12 @@ ModulesBuilder::buildPrerequisiteModulesFor(PathRef File, auto RequiredModules = std::make_unique<ReusablePrerequisiteModules>(); for (llvm::StringRef RequiredModuleName : RequiredModuleNames) { + // Return early if there is any error. if (llvm::Error Err = Impl->getOrBuildModuleFile( File, RequiredModuleName, TFS, CachedMDB, *RequiredModules.get())) { elog("Failed to build module {0}; due to {1}", RequiredModuleName, toString(std::move(Err))); - RequiredModules->setHasFailures(); + return std::make_unique<FailedPrerequisiteModules>(); } } diff --git a/clang/lib/Parse/Parser.cpp b/clang/lib/Parse/Parser.cpp index 5d87453cf219e..5e1fd4df1a3f0 100644 --- a/clang/lib/Parse/Parser.cpp +++ b/clang/lib/Parse/Parser.cpp @@ -2451,7 +2451,13 @@ Decl *Parser::ParseModuleImport(SourceLocation AtLoc, /*DiagnoseEmptyAttrs=*/false, /*WarnOnUnknownAttrs=*/true); - if (PP.hadModuleLoaderFatalFailure()) { + // Clang modules can inject token streams while loading, so a fatal loader + // failure must stop parsing. C++20 named module imports are ordinary + // declarations, and a prior failed import should not hide later diagnostics. + bool IsCXX20NamedModuleImport = + getLangOpts().CPlusPlusModules && !IsObjCAtImport && !Path.empty(); + + if (PP.hadModuleLoaderFatalFailure() && !IsCXX20NamedModuleImport) { // With a fatal failure in the module loader, we abort parsing. cutOffParsing(); return nullptr; diff --git a/clang/lib/Tooling/Syntax/Tokens.cpp b/clang/lib/Tooling/Syntax/Tokens.cpp index 9d9d779e931ca..927333fda18bb 100644 --- a/clang/lib/Tooling/Syntax/Tokens.cpp +++ b/clang/lib/Tooling/Syntax/Tokens.cpp @@ -910,7 +910,6 @@ class TokenCollector::Builder { TokenBuffer TokenCollector::consume() && { PP.setTokenWatcher(nullptr); Collector->disable(); - return Builder(std::move(Expanded), std::move(Expansions), PP.getSourceManager(), PP.getLangOpts()) .build(); diff --git a/clang/test/Modules/cxx20-fatal-module-loader-error.cpp b/clang/test/Modules/cxx20-fatal-module-loader-error.cpp new file mode 100644 index 0000000000000..ea2b3a2274fd3 --- /dev/null +++ b/clang/test/Modules/cxx20-fatal-module-loader-error.cpp @@ -0,0 +1,12 @@ +// RUN: rm -rf %t +// RUN: mkdir %t +// RUN: touch %t/A.pcm +// RUN: not %clang_cc1 -std=c++20 -fmodule-file=A=%t/A.pcm -ast-dump %s 2>&1 | FileCheck %s + +// CHECK: fatal error: file '{{.*}}A.pcm' is not a valid module file +// CHECK: VarDecl {{.*}} n 'int' + +import A; +import NonExistent; + +int n; _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
