sammccall created this revision.
sammccall added a reviewer: kadircet.
Herald added a subscriber: arphaman.
Herald added a project: All.
sammccall requested review of this revision.
Herald added subscribers: cfe-commits, MaskRay, ilya-biryukov.
Herald added a project: clang-tools-extra.
We don't yet have any intentional modules support:
https://github.com/clangd/clangd/issues/1293
However if the user has modules-related flags in their CDB, modules will still
be enabled inside clang.
- for some configurations this works well enough, apparently. We should allow
them to keep using this config if they wish, though unsupported.
- for other configurations this is crashy/buggy. We can't really control the
behavior/stability, so long-term this should not be the default.
- there is no flag to disable modules if when using `-std=c++20`. Since clangd
does not support modules, we should provide a mechanism.
- in future we're likely to explicitly add modules support. This will not be a
simple passthrough of flags to clang, so we'll need another mode.
This patch adds a config option:
CompileFlags:
NaiveModules: false
For now, the default is true, and modules are internally enabled/disabled
according to compile flags. When set to false, we force Modules/CPlusPlusModules
lang opts off.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D155392
Files:
clang-tools-extra/clangd/Compiler.cpp
clang-tools-extra/clangd/Config.h
clang-tools-extra/clangd/ConfigCompile.cpp
clang-tools-extra/clangd/ConfigFragment.h
clang-tools-extra/clangd/ConfigYAML.cpp
clang-tools-extra/clangd/unittests/FindSymbolsTests.cpp
clang-tools-extra/clangd/unittests/ModulesTests.cpp
clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp
Index: clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp
===================================================================
--- clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp
+++ clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp
@@ -7,9 +7,11 @@
//===----------------------------------------------------------------------===//
#include "Annotations.h"
+#include "Config.h"
#include "TestFS.h"
#include "TestTU.h"
#include "index/SymbolCollector.h"
+#include "support/Context.h"
#include "clang/Basic/FileManager.h"
#include "clang/Basic/FileSystemOptions.h"
#include "clang/Frontend/CompilerInstance.h"
@@ -28,6 +30,7 @@
#include <memory>
#include <optional>
#include <string>
+#include <utility>
namespace clang {
namespace clangd {
@@ -1918,6 +1921,10 @@
// Regression test for a crash-bug we used to have.
TEST_F(SymbolCollectorTest, UndefOfModuleMacro) {
+ Config EnableModules;
+ EnableModules.CompileFlags.NaiveModules = true;
+ WithContextValue WithCfg(Config::Key, std::move(EnableModules));
+
auto TU = TestTU::withCode(R"cpp(#include "bar.h")cpp");
TU.AdditionalFiles["bar.h"] = R"cpp(
#include "foo.h"
Index: clang-tools-extra/clangd/unittests/ModulesTests.cpp
===================================================================
--- clang-tools-extra/clangd/unittests/ModulesTests.cpp
+++ clang-tools-extra/clangd/unittests/ModulesTests.cpp
@@ -6,8 +6,10 @@
//
//===----------------------------------------------------------------------===//
+#include "Config.h"
#include "TestFS.h"
#include "TestTU.h"
+#include "support/Context.h"
#include "gmock/gmock.h"
#include "gtest/gtest.h"
@@ -18,7 +20,21 @@
namespace clangd {
namespace {
-TEST(Modules, TextualIncludeInPreamble) {
+enum class ModulesMode { Disabled, Naive };
+Config makeModuleConfig(ModulesMode M) {
+ Config EnableModules;
+ EnableModules.CompileFlags.NaiveModules = M == ModulesMode::Naive;
+ return EnableModules;
+}
+
+class ModulesTest : public ::testing::TestWithParam<ModulesMode> {
+ WithContextValue WithCfg;
+
+protected:
+ ModulesTest() : WithCfg(Config::Key, makeModuleConfig(GetParam())) {}
+};
+
+TEST_P(ModulesTest, TextualIncludeInPreamble) {
TestTU TU = TestTU::withCode(R"cpp(
#include "Textual.h"
@@ -40,7 +56,7 @@
// Verify that visibility of AST nodes belonging to modules, but loaded from
// preamble PCH, is restored.
-TEST(Modules, PreambleBuildVisibility) {
+TEST_P(ModulesTest, PreambleBuildVisibility) {
TestTU TU = TestTU::withCode(R"cpp(
#include "module.h"
@@ -63,7 +79,7 @@
EXPECT_TRUE(TU.build().getDiagnostics().empty());
}
-TEST(Modules, Diagnostic) {
+TEST_P(ModulesTest, Diagnostic) {
// Produce a diagnostic while building an implicit module. Use
// -fmodules-strict-decluse, but any non-silenced diagnostic will do.
TestTU TU = TestTU::withCode(R"cpp(
@@ -92,7 +108,7 @@
}
// Unknown module formats are a fatal failure for clang. Ensure we don't crash.
-TEST(Modules, UnknownFormat) {
+TEST_P(ModulesTest, UnknownFormat) {
TestTU TU = TestTU::withCode(R"(#include "modular.h")");
TU.OverlayRealFileSystemForModules = true;
TU.ExtraArgs.push_back("-Xclang");
@@ -109,6 +125,25 @@
// Test that we do not crash.
TU.build();
}
+
+MATCHER_P(diag, M, "") { return arg.Message == M; }
+
+TEST_P(ModulesTest, Cpp20Import) {
+ TestTU TU = TestTU::withCode("import foo; // error-ok");
+ TU.ExtraArgs.push_back("-Xclang");
+ TU.ExtraArgs.push_back("-std=c++20");
+
+ auto AST = TU.build();
+ EXPECT_THAT(AST.getDiagnostics(),
+ testing::ElementsAre(GetParam() == ModulesMode::Naive
+ ? diag("module 'foo' not found")
+ : diag("unknown type name 'import'")));
+}
+
+INSTANTIATE_TEST_SUITE_P(ModulesTestModes, ModulesTest,
+ testing::Values(ModulesMode::Disabled,
+ ModulesMode::Naive));
+
} // namespace
} // namespace clangd
} // namespace clang
Index: clang-tools-extra/clangd/unittests/FindSymbolsTests.cpp
===================================================================
--- clang-tools-extra/clangd/unittests/FindSymbolsTests.cpp
+++ clang-tools-extra/clangd/unittests/FindSymbolsTests.cpp
@@ -6,12 +6,15 @@
//
//===----------------------------------------------------------------------===//
#include "Annotations.h"
+#include "Config.h"
#include "FindSymbols.h"
#include "TestFS.h"
#include "TestTU.h"
+#include "support/Context.h"
#include "llvm/ADT/StringRef.h"
#include "gmock/gmock.h"
#include "gtest/gtest.h"
+#include <utility>
namespace clang {
namespace clangd {
@@ -491,6 +494,10 @@
}
TEST(DocumentSymbols, ExportContext) {
+ Config EnableModules;
+ EnableModules.CompileFlags.NaiveModules = true;
+ WithContextValue WithCfg(Config::Key, std::move(EnableModules));
+
TestTU TU;
TU.ExtraArgs = {"-std=c++20"};
TU.Code = R"cpp(
Index: clang-tools-extra/clangd/ConfigYAML.cpp
===================================================================
--- clang-tools-extra/clangd/ConfigYAML.cpp
+++ clang-tools-extra/clangd/ConfigYAML.cpp
@@ -108,6 +108,9 @@
Dict.handle("CompilationDatabase", [&](Node &N) {
F.CompilationDatabase = scalarValue(N, "CompilationDatabase");
});
+ Dict.handle("NaiveModules", [&](Node &N) {
+ F.NaiveModules = boolValue(N, "NaiveModules");
+ });
Dict.parse(N);
}
Index: clang-tools-extra/clangd/ConfigFragment.h
===================================================================
--- clang-tools-extra/clangd/ConfigFragment.h
+++ clang-tools-extra/clangd/ConfigFragment.h
@@ -169,6 +169,11 @@
/// - Ancestors: search all parent directories (the default)
/// - std::nullopt: do not use a compilation database, just default flags.
std::optional<Located<std::string>> CompilationDatabase;
+
+ /// Allow flags such as -fmodules or -std=c++20 to enable modules.
+ /// We don't support modules: https://github.com/clangd/clangd/issues/1293
+ /// However some configurations may happen to work anyway.
+ std::optional<Located<bool>> NaiveModules;
};
CompileFlagsBlock CompileFlags;
Index: clang-tools-extra/clangd/ConfigCompile.cpp
===================================================================
--- clang-tools-extra/clangd/ConfigCompile.cpp
+++ clang-tools-extra/clangd/ConfigCompile.cpp
@@ -320,6 +320,12 @@
C.CompileFlags.CDBSearch = Spec;
});
}
+
+ if (F.NaiveModules)
+ Out.Apply.push_back(
+ [NaiveModules(**F.NaiveModules)](const Params &, Config &C) {
+ C.CompileFlags.NaiveModules = NaiveModules;
+ });
}
void compile(Fragment::IndexBlock &&F) {
Index: clang-tools-extra/clangd/Config.h
===================================================================
--- clang-tools-extra/clangd/Config.h
+++ clang-tools-extra/clangd/Config.h
@@ -66,6 +66,8 @@
Edits;
/// Where to search for compilation databases for this file's flags.
CDBSearchSpec CDBSearch = {CDBSearchSpec::Ancestors, std::nullopt};
+ /// Allow modules when enabled by -fmodules, -std=c++20, etc.
+ bool NaiveModules = true;
} CompileFlags;
enum class BackgroundPolicy { Build, Skip };
Index: clang-tools-extra/clangd/Compiler.cpp
===================================================================
--- clang-tools-extra/clangd/Compiler.cpp
+++ clang-tools-extra/clangd/Compiler.cpp
@@ -7,6 +7,7 @@
//===----------------------------------------------------------------------===//
#include "Compiler.h"
+#include "Config.h"
#include "support/Logger.h"
#include "clang/Basic/TargetInfo.h"
#include "clang/Frontend/CompilerInvocation.h"
@@ -89,6 +90,11 @@
CI.getLangOpts()->ProfileListFiles.clear();
CI.getLangOpts()->XRayAlwaysInstrumentFiles.clear();
CI.getLangOpts()->XRayNeverInstrumentFiles.clear();
+
+ if (!Config::current().CompileFlags.NaiveModules) {
+ CI.getLangOpts()->Modules = false;
+ CI.getLangOpts()->CPlusPlusModules = false;
+ }
}
std::unique_ptr<CompilerInvocation>
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
