Skip to content

Commit 23459f1

Browse files
committed
[Lex] Preambles should contain the global module fragment.
For applications like clangd, the preamble remains an important optimization when editing a module definition. The global module fragment is a good fit for it as it by definition contains only preprocessor directives. Before this patch, we would terminate the preamble immediately at the "module" keyword. Differential Revision: https://reviews.llvm.org/D158439
1 parent 5f6c036 commit 23459f1

File tree

4 files changed

+80
-0
lines changed

4 files changed

+80
-0
lines changed

clang-tools-extra/clangd/unittests/ModulesTests.cpp

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88

99
#include "TestFS.h"
1010
#include "TestTU.h"
11+
#include "llvm/ADT/StringRef.h"
1112
#include "gmock/gmock.h"
1213
#include "gtest/gtest.h"
1314

@@ -109,6 +110,34 @@ TEST(Modules, UnknownFormat) {
109110
// Test that we do not crash.
110111
TU.build();
111112
}
113+
114+
// Test that we can build and use a preamble for a module unit.
115+
TEST(Modules, ModulePreamble) {
116+
TestTU TU = TestTU::withCode(R"cpp(
117+
module;
118+
#define PREAMBLE_MACRO 1
119+
export module foo;
120+
#define MODULE_MACRO 2
121+
module :private;
122+
#define PRIVATE_MACRO 3
123+
)cpp");
124+
TU.ExtraArgs.push_back("-std=c++20");
125+
TU.ExtraArgs.push_back("--precompile");
126+
127+
auto AST = TU.build();
128+
auto &SM = AST.getSourceManager();
129+
auto GetMacroFile = [&](llvm::StringRef Name) -> FileID {
130+
if (auto *MI = AST.getPreprocessor().getMacroInfo(
131+
&AST.getASTContext().Idents.get(Name)))
132+
return SM.getFileID(MI->getDefinitionLoc());
133+
return {};
134+
};
135+
136+
EXPECT_EQ(GetMacroFile("PREAMBLE_MACRO"), SM.getPreambleFileID());
137+
EXPECT_EQ(GetMacroFile("MODULE_MACRO"), SM.getMainFileID());
138+
EXPECT_EQ(GetMacroFile("PRIVATE_MACRO"), SM.getMainFileID());
139+
}
140+
112141
} // namespace
113142
} // namespace clangd
114143
} // namespace clang

clang/lib/Lex/Lexer.cpp

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -705,6 +705,22 @@ PreambleBounds Lexer::ComputePreamble(StringRef Buffer,
705705
// directive or it was one that can't occur in the preamble at this
706706
// point. Roll back the current token to the location of the '#'.
707707
TheTok = HashTok;
708+
} else if (TheTok.isAtStartOfLine() &&
709+
TheTok.getKind() == tok::raw_identifier &&
710+
TheTok.getRawIdentifier() == "module" &&
711+
LangOpts.CPlusPlusModules) {
712+
// The initial global module fragment introducer "module;" is part of
713+
// the preamble, which runs up to the module declaration "module foo;".
714+
Token ModuleTok = TheTok;
715+
do {
716+
TheLexer.LexFromRawLexer(TheTok);
717+
} while (TheTok.getKind() == tok::comment);
718+
if (TheTok.getKind() != tok::semi) {
719+
// Not global module fragment, roll back.
720+
TheTok = ModuleTok;
721+
break;
722+
}
723+
continue;
708724
}
709725

710726
// We hit a token that we don't recognize as being in the

clang/unittests/Lex/CMakeLists.txt

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,5 +25,6 @@ clang_target_link_libraries(LexTests
2525

2626
target_link_libraries(LexTests
2727
PRIVATE
28+
LLVMTestingAnnotations
2829
LLVMTestingSupport
2930
)

clang/unittests/Lex/LexerTest.cpp

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,9 +26,11 @@
2626
#include "clang/Lex/PreprocessorOptions.h"
2727
#include "llvm/ADT/ArrayRef.h"
2828
#include "llvm/ADT/StringRef.h"
29+
#include "llvm/Testing/Annotations/Annotations.h"
2930
#include "gmock/gmock.h"
3031
#include "gtest/gtest.h"
3132
#include <memory>
33+
#include <string>
3234
#include <vector>
3335

3436
namespace {
@@ -660,4 +662,36 @@ TEST_F(LexerTest, RawAndNormalLexSameForLineComments) {
660662
}
661663
EXPECT_TRUE(ToksView.empty());
662664
}
665+
666+
TEST(LexerPreambleTest, PreambleBounds) {
667+
std::vector<std::string> Cases = {
668+
R"cc([[
669+
#include <foo>
670+
]]int bar;
671+
)cc",
672+
R"cc([[
673+
#include <foo>
674+
]])cc",
675+
R"cc([[
676+
// leading comment
677+
#include <foo>
678+
]]// trailing comment
679+
int bar;
680+
)cc",
681+
R"cc([[
682+
module;
683+
#include <foo>
684+
]]module bar;
685+
int x;
686+
)cc",
687+
};
688+
for (const auto& Case : Cases) {
689+
llvm::Annotations A(Case);
690+
clang::LangOptions LangOpts;
691+
LangOpts.CPlusPlusModules = true;
692+
auto Bounds = Lexer::ComputePreamble(A.code(), LangOpts);
693+
EXPECT_EQ(Bounds.Size, A.range().End) << Case;
694+
}
695+
}
696+
663697
} // anonymous namespace

0 commit comments

Comments
 (0)