kadircet created this revision.
Herald added subscribers: cfe-commits, usaxena95, arphaman, jkorous, MaskRay,
ilya-biryukov.
Herald added a project: clang.
Depends on D79992 <https://reviews.llvm.org/D79992>.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D80198
Files:
clang-tools-extra/clangd/Headers.cpp
clang-tools-extra/clangd/Hover.cpp
clang-tools-extra/clangd/Preamble.cpp
clang-tools-extra/clangd/SourceCode.cpp
clang-tools-extra/clangd/SourceCode.h
clang-tools-extra/clangd/XRefs.cpp
clang-tools-extra/clangd/refactor/Rename.cpp
clang-tools-extra/clangd/unittests/CollectMacrosTests.cpp
clang-tools-extra/clangd/unittests/PreambleTests.cpp
clang-tools-extra/clangd/unittests/SourceCodeTests.cpp
Index: clang-tools-extra/clangd/unittests/SourceCodeTests.cpp
===================================================================
--- clang-tools-extra/clangd/unittests/SourceCodeTests.cpp
+++ clang-tools-extra/clangd/unittests/SourceCodeTests.cpp
@@ -514,7 +514,7 @@
ASSERT_TRUE(bool(CurLoc));
const auto *Id = syntax::spelledIdentifierTouching(*CurLoc, AST.getTokens());
ASSERT_TRUE(Id);
- auto Result = locateMacroAt(*Id, AST.getPreprocessor());
+ auto Result = locateMacroAt(*Id, AST.getPreprocessor(), AST.getTokens());
ASSERT_TRUE(Result);
EXPECT_THAT(*Result, MacroName("MACRO"));
}
@@ -528,7 +528,7 @@
ASSERT_TRUE(bool(CurLoc));
const auto *Id = syntax::spelledIdentifierTouching(*CurLoc, AST.getTokens());
ASSERT_TRUE(Id);
- auto Result = locateMacroAt(*Id, AST.getPreprocessor());
+ auto Result = locateMacroAt(*Id, AST.getPreprocessor(), AST.getTokens());
ASSERT_TRUE(Result);
EXPECT_THAT(*Result, MacroName("MACRO"));
}
Index: clang-tools-extra/clangd/unittests/PreambleTests.cpp
===================================================================
--- clang-tools-extra/clangd/unittests/PreambleTests.cpp
+++ clang-tools-extra/clangd/unittests/PreambleTests.cpp
@@ -198,7 +198,7 @@
ADD_FAILURE() << "Failed to build compiler invocation";
return llvm::None;
}
- return ParsedAST::build(testPath("main.cpp"), TU.inputs(), std::move(CI), {},
+ return ParsedAST::build(testPath(TU.Filename), TU.inputs(), std::move(CI), {},
BaselinePreamble);
}
@@ -243,6 +243,101 @@
EXPECT_THAT(AST->getDiagnostics(),
Not(Contains(Field(&Diag::Range, Modified.range()))));
}
+
+TEST(PreamblePatchTest, LocateMacroAtWorks) {
+ struct {
+ llvm::StringLiteral Baseline;
+ llvm::StringLiteral Modified;
+ } Cases[] = {
+ // Addition of new directive
+ {
+ "",
+ R"cpp(
+ #define $def^FOO
+ $use^FOO)cpp",
+ },
+ // Available inside preamble section
+ {
+ "",
+ R"cpp(
+ #define $def^FOO
+ #undef $use^FOO)cpp",
+ },
+ // Available after undef, as we don't patch those
+ {
+ "",
+ R"cpp(
+ #define $def^FOO
+ #undef FOO
+ $use^FOO)cpp",
+ },
+ // Identifier on a different line
+ {
+ "",
+ R"cpp(
+ #define \
+ $def^FOO
+ $use^FOO)cpp",
+ },
+ // In presence of comment tokens
+ {
+ "",
+ R"cpp(
+ #\
+ define /* FOO */\
+ /* FOO */ $def^FOO
+ $use^FOO)cpp",
+ },
+ // Moved around
+ {
+ "#define FOO",
+ R"cpp(
+ #define BAR
+ #define $def^FOO
+ $use^FOO)cpp",
+ },
+ };
+ for (const auto &Case : Cases) {
+ SCOPED_TRACE(Case.Modified);
+ llvm::Annotations Modified(Case.Modified);
+ auto AST = createPatchedAST(Case.Baseline, Modified.code());
+ ASSERT_TRUE(AST);
+
+ const auto &SM = AST->getSourceManager();
+ auto *MacroTok = AST->getTokens().spelledTokenAt(
+ SM.getComposedLoc(SM.getMainFileID(), Modified.point("use")));
+ ASSERT_TRUE(MacroTok);
+
+ auto FoundMacro =
+ locateMacroAt(*MacroTok, AST->getPreprocessor(), AST->getTokens());
+ ASSERT_TRUE(FoundMacro);
+ EXPECT_THAT(FoundMacro->Name, "FOO");
+
+ auto MacroLoc = FoundMacro->DefRange.getBegin();
+ EXPECT_EQ(SM.getFileID(MacroLoc), SM.getMainFileID());
+ EXPECT_EQ(SM.getFileOffset(MacroLoc), Modified.point("def"));
+ }
+}
+
+TEST(PreamblePatchTest, LocateMacroAtDeletion) {
+ // We don't patch deleted define directives, make sure we don't crash.
+ llvm::StringLiteral Baseline = "#define FOO";
+ llvm::Annotations Modified("^FOO");
+
+ auto AST = createPatchedAST(Baseline, Modified.code());
+ ASSERT_TRUE(AST);
+
+ const auto &SM = AST->getSourceManager();
+ auto *MacroTok = AST->getTokens().spelledTokenAt(
+ SM.getComposedLoc(SM.getMainFileID(), Modified.point()));
+ ASSERT_TRUE(MacroTok);
+
+ auto FoundMacro =
+ locateMacroAt(*MacroTok, AST->getPreprocessor(), AST->getTokens());
+ ASSERT_TRUE(FoundMacro);
+ EXPECT_THAT(FoundMacro->Name, "FOO");
+}
+
} // namespace
} // namespace clangd
} // namespace clang
Index: clang-tools-extra/clangd/unittests/CollectMacrosTests.cpp
===================================================================
--- clang-tools-extra/clangd/unittests/CollectMacrosTests.cpp
+++ clang-tools-extra/clangd/unittests/CollectMacrosTests.cpp
@@ -91,7 +91,7 @@
ASSERT_TRUE(bool(Loc));
const auto *Id = syntax::spelledIdentifierTouching(*Loc, AST.getTokens());
ASSERT_TRUE(Id);
- auto Macro = locateMacroAt(*Id, PP);
+ auto Macro = locateMacroAt(*Id, PP, AST.getTokens());
assert(Macro);
auto SID = getSymbolID(Macro->Name, Macro->DefRange.getBegin(), SM);
assert(SID);
Index: clang-tools-extra/clangd/refactor/Rename.cpp
===================================================================
--- clang-tools-extra/clangd/refactor/Rename.cpp
+++ clang-tools-extra/clangd/refactor/Rename.cpp
@@ -477,7 +477,7 @@
return makeError(ReasonToReject::NoSymbolFound);
// FIXME: Renaming macros is not supported yet, the macro-handling code should
// be moved to rename tooling library.
- if (locateMacroAt(*IdentifierToken, AST.getPreprocessor()))
+ if (locateMacroAt(*IdentifierToken, AST.getPreprocessor(), AST.getTokens()))
return makeError(ReasonToReject::UnsupportedSymbol);
auto DeclsUnderCursor = locateDeclAt(AST, IdentifierToken->location());
Index: clang-tools-extra/clangd/XRefs.cpp
===================================================================
--- clang-tools-extra/clangd/XRefs.cpp
+++ clang-tools-extra/clangd/XRefs.cpp
@@ -205,7 +205,8 @@
llvm::Optional<LocatedSymbol>
locateMacroReferent(const syntax::Token &TouchedIdentifier, ParsedAST &AST,
llvm::StringRef MainFilePath) {
- if (auto M = locateMacroAt(TouchedIdentifier, AST.getPreprocessor())) {
+ if (auto M = locateMacroAt(TouchedIdentifier, AST.getPreprocessor(),
+ AST.getTokens())) {
if (auto Loc = makeLocation(AST.getASTContext(), M->DefRange.getBegin(),
MainFilePath)) {
LocatedSymbol Macro;
@@ -749,7 +750,8 @@
llvm::Optional<DefinedMacro> Macro;
if (const auto *IdentifierAtCursor =
syntax::spelledIdentifierTouching(*CurLoc, AST.getTokens())) {
- Macro = locateMacroAt(*IdentifierAtCursor, AST.getPreprocessor());
+ Macro = locateMacroAt(*IdentifierAtCursor, AST.getPreprocessor(),
+ AST.getTokens());
}
RefsRequest Req;
@@ -871,7 +873,8 @@
if (!IdentifierAtCursor)
return Results;
- if (auto M = locateMacroAt(*IdentifierAtCursor, AST.getPreprocessor())) {
+ if (auto M = locateMacroAt(*IdentifierAtCursor, AST.getPreprocessor(),
+ AST.getTokens())) {
SymbolDetails NewMacro;
NewMacro.name = std::string(M->Name);
llvm::SmallString<32> USR;
Index: clang-tools-extra/clangd/SourceCode.h
===================================================================
--- clang-tools-extra/clangd/SourceCode.h
+++ clang-tools-extra/clangd/SourceCode.h
@@ -296,7 +296,8 @@
/// Gets the macro referenced by \p SpelledTok. It must be a spelled token
/// aligned to the beginning of an identifier.
llvm::Optional<DefinedMacro> locateMacroAt(const syntax::Token &SpelledTok,
- Preprocessor &PP);
+ Preprocessor &PP,
+ const syntax::TokenBuffer &TB);
/// Infers whether this is a header from the FileName and LangOpts (if
/// presents).
@@ -306,6 +307,9 @@
/// Returns true if the given location is in a generated protobuf file.
bool isProtoFile(SourceLocation Loc, const SourceManager &SourceMgr);
+/// Checks whether \p FileName is a valid spelling of main file.
+bool isMainFile(llvm::StringRef FileName, const SourceManager &SM);
+
} // namespace clangd
} // namespace clang
#endif
Index: clang-tools-extra/clangd/SourceCode.cpp
===================================================================
--- clang-tools-extra/clangd/SourceCode.cpp
+++ clang-tools-extra/clangd/SourceCode.cpp
@@ -945,8 +945,14 @@
return Result;
}
+bool isMainFile(llvm::StringRef FileName, const SourceManager &SM) {
+ auto FE = SM.getFileManager().getFile(FileName);
+ return FE && *FE == SM.getFileEntryForID(SM.getMainFileID());
+}
+
llvm::Optional<DefinedMacro> locateMacroAt(const syntax::Token &SpelledTok,
- Preprocessor &PP) {
+ Preprocessor &PP,
+ const syntax::TokenBuffer &TB) {
SourceLocation Loc = SpelledTok.location();
assert(Loc.isFileID());
const auto &SM = PP.getSourceManager();
@@ -960,13 +966,45 @@
if (SM.getLocForStartOfFile(SM.getFileID(Loc)) != Loc)
Loc = Loc.getLocWithOffset(-1);
MacroDefinition MacroDef = PP.getMacroDefinitionAtLoc(IdentifierInfo, Loc);
- if (auto *MI = MacroDef.getMacroInfo())
- return DefinedMacro{
- IdentifierInfo->getName(),
- // MacroInfo::getDefinitionEndLoc returns the location for last token.
- CharSourceRange::getTokenRange(MI->getDefinitionLoc(),
- MI->getDefinitionEndLoc())};
- return None;
+ auto *MI = MacroDef.getMacroInfo();
+ if (!MI)
+ return None;
+
+ SourceRange DefRange(MI->getDefinitionLoc(), MI->getDefinitionEndLoc());
+
+ // Macro definitions could be injected through preamble patch. These contain
+ // line directives to hint their original location in main file.
+ auto DefFile = SM.getFileID(DefRange.getBegin());
+ auto IncludeLoc = SM.getIncludeLoc(DefFile);
+ // Preamble patch is included inside the builtin file.
+ if (IncludeLoc.isValid() && SM.isWrittenInBuiltinFile(IncludeLoc)) {
+ auto Presumed = SM.getPresumedLoc(DefRange.getBegin());
+ // Check that line directive is pointing at main file.
+ if (Presumed.isValid() && isMainFile(Presumed.getFilename(), SM)) {
+ auto Presumed = SM.getPresumedLoc(DefRange.getBegin());
+ // Now find the spelled token for macro identifier. The line looks like:
+ // # /*comment*/ define /*comment*/ MACRO_IDENT....
+ auto LocForStartOfLine =
+ SM.translateLineCol(SM.getMainFileID(), Presumed.getLine(), 1);
+ auto SpelledToks = TB.spelledTokens(SM.getMainFileID());
+ auto *MacroIdent = llvm::partition_point(
+ SpelledToks, [&LocForStartOfLine](const syntax::Token &Tok) {
+ return Tok.location() < LocForStartOfLine;
+ });
+ assert(MacroIdent);
+ while (MacroIdent->text(SM) != SpelledTok.text(SM))
+ ++MacroIdent;
+ assert(MacroIdent < SpelledToks.end());
+
+ auto Delta = SM.getFileOffset(DefRange.getEnd()) -
+ SM.getFileOffset(DefRange.getBegin());
+ DefRange.setBegin(MacroIdent->location());
+ DefRange.setEnd(DefRange.getBegin().getLocWithOffset(Delta));
+ }
+ }
+
+ return DefinedMacro{IdentifierInfo->getName(),
+ CharSourceRange::getTokenRange(DefRange)};
}
llvm::Expected<std::string> Edit::apply() const {
Index: clang-tools-extra/clangd/Preamble.cpp
===================================================================
--- clang-tools-extra/clangd/Preamble.cpp
+++ clang-tools-extra/clangd/Preamble.cpp
@@ -427,8 +427,10 @@
// Note that we deliberately ignore conditional directives and undefs to
// reduce complexity. The former might cause problems because scanning is
// imprecise and might pick directives from disabled regions.
- for (const auto &TD : ModifiedScan->TextualDirectives)
+ for (const auto &TD : ModifiedScan->TextualDirectives) {
+ Patch << "#line " << TD.DirectiveLine << '\n';
Patch << TD.Text << '\n';
+ }
}
dlog("Created preamble patch: {0}", Patch.str());
Patch.flush();
Index: clang-tools-extra/clangd/Hover.cpp
===================================================================
--- clang-tools-extra/clangd/Hover.cpp
+++ clang-tools-extra/clangd/Hover.cpp
@@ -704,7 +704,7 @@
if (Tok.kind() == tok::identifier) {
// Prefer the identifier token as a fallback highlighting range.
HighlightRange = Tok.range(SM).toCharRange(SM);
- if (auto M = locateMacroAt(Tok, AST.getPreprocessor())) {
+ if (auto M = locateMacroAt(Tok, AST.getPreprocessor(), AST.getTokens())) {
HI = getHoverContents(*M, AST);
break;
}
Index: clang-tools-extra/clangd/Headers.cpp
===================================================================
--- clang-tools-extra/clangd/Headers.cpp
+++ clang-tools-extra/clangd/Headers.cpp
@@ -23,11 +23,6 @@
namespace clangd {
namespace {
-bool isMainFile(llvm::StringRef FileName, const SourceManager &SM) {
- auto FE = SM.getFileManager().getFile(FileName);
- return FE && *FE == SM.getFileEntryForID(SM.getMainFileID());
-}
-
class RecordHeaders : public PPCallbacks {
public:
RecordHeaders(const SourceManager &SM, IncludeStructure *Out)
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
