diff --git a/clang-tools-extra/clangd/Headers.cpp b/clang-tools-extra/clangd/Headers.cpp --- a/clang-tools-extra/clangd/Headers.cpp +++ b/clang-tools-extra/clangd/Headers.cpp @@ -8,6 +8,7 @@ #include "Headers.h" #include "Compiler.h" +#include "Preamble.h" #include "SourceCode.h" #include "support/Logger.h" #include "clang/Basic/SourceLocation.h" @@ -23,11 +24,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) @@ -44,17 +40,8 @@ SrcMgr::CharacteristicKind FileKind) override { auto MainFID = SM.getMainFileID(); // If an include is part of the preamble patch, translate #line directives. - if (InBuiltinFile) { - auto Presumed = SM.getPresumedLoc(HashLoc); - // Presumed locations will have an invalid file id when #line directive - // changes the filename. - if (Presumed.getFileID().isInvalid() && - isMainFile(Presumed.getFilename(), SM)) { - // Now we'll hit the case below. - HashLoc = SM.translateLineCol(MainFID, Presumed.getLine(), - Presumed.getColumn()); - } - } + if (InBuiltinFile) + HashLoc = translatePreamblePatchLocation(HashLoc, SM); // Record main-file inclusions (including those mapped from the preamble // patch). diff --git a/clang-tools-extra/clangd/Hover.cpp b/clang-tools-extra/clangd/Hover.cpp --- a/clang-tools-extra/clangd/Hover.cpp +++ b/clang-tools-extra/clangd/Hover.cpp @@ -376,8 +376,7 @@ const auto *ME = llvm::dyn_cast(E->IgnoreCasts()); if (!ME || !llvm::isa(ME->getBase()->IgnoreCasts())) return llvm::None; - const auto *Field = - llvm::dyn_cast(ME->getMemberDecl()); + const auto *Field = llvm::dyn_cast(ME->getMemberDecl()); if (!Field || !Field->getDeclName().isIdentifier()) return llvm::None; return Field->getDeclName().getAsIdentifierInfo()->getName(); @@ -555,7 +554,14 @@ // Try to get the full definition, not just the name SourceLocation StartLoc = Macro.Info->getDefinitionLoc(); SourceLocation EndLoc = Macro.Info->getDefinitionEndLoc(); - if (EndLoc.isValid()) { + // Ensure that EndLoc is a valid offset. For example it might come from + // preamble, and source file might've changed, in such a scenario EndLoc still + // stays valid, but getLocForEndOfToken will fail as it is no longer a valid + // offset. + // Note that this check is just to ensure there's text data inside the range. + // It will still succeed even when the data inside the range is irrelevant to + // macro definition. + if (SM.getPresumedLoc(EndLoc, /*UseLineDirectives=*/false).isValid()) { EndLoc = Lexer::getLocForEndOfToken(EndLoc, 0, SM, AST.getLangOpts()); bool Invalid; StringRef Buffer = SM.getBufferData(SM.getFileID(StartLoc), &Invalid); @@ -869,20 +875,20 @@ if (!Suffix.empty() && !AfterEndChars.contains(Suffix.front())) return llvm::None; - return Line.slice(Offset, Next+1); + return Line.slice(Offset, Next + 1); } void parseDocumentationLine(llvm::StringRef Line, markup::Paragraph &Out) { // Probably this is appendText(Line), but scan for something interesting. for (unsigned I = 0; I < Line.size(); ++I) { switch (Line[I]) { - case '`': - if (auto Range = getBacktickQuoteRange(Line, I)) { - Out.appendText(Line.substr(0, I)); - Out.appendCode(Range->trim("`"), /*Preserve=*/true); - return parseDocumentationLine(Line.substr(I+Range->size()), Out); - } - break; + case '`': + if (auto Range = getBacktickQuoteRange(Line, I)) { + Out.appendText(Line.substr(0, I)); + Out.appendCode(Range->trim("`"), /*Preserve=*/true); + return parseDocumentationLine(Line.substr(I + Range->size()), Out); + } + break; } } Out.appendText(Line).appendSpace(); diff --git a/clang-tools-extra/clangd/Preamble.h b/clang-tools-extra/clangd/Preamble.h --- a/clang-tools-extra/clangd/Preamble.h +++ b/clang-tools-extra/clangd/Preamble.h @@ -131,6 +131,11 @@ std::vector PreambleIncludes; }; +/// Translates locations inside preamble patch to their main-file equivalent +/// using presumed locations. Returns \p Loc if it isn't inside preamble patch. +SourceLocation translatePreamblePatchLocation(SourceLocation Loc, + const SourceManager &SM); + } // namespace clangd } // namespace clang diff --git a/clang-tools-extra/clangd/Preamble.cpp b/clang-tools-extra/clangd/Preamble.cpp --- a/clang-tools-extra/clangd/Preamble.cpp +++ b/clang-tools-extra/clangd/Preamble.cpp @@ -50,6 +50,7 @@ namespace clang { namespace clangd { namespace { +constexpr llvm::StringLiteral PreamblePatchHeaderName = "__preamble_patch__.h"; bool compileCommandsAreEqual(const tooling::CompileCommand &LHS, const tooling::CompileCommand &RHS) { @@ -120,6 +121,49 @@ } }; +// Formats a PP directive consisting of Prefix (e.g. "#define ") and Body ("X +// 10"). The formatting is copied so that the tokens in Body have PresumedLocs +// with correct columns and lines. +std::string spellDirective(llvm::StringRef Prefix, + CharSourceRange DirectiveRange, + const LangOptions &LangOpts, const SourceManager &SM, + unsigned &DirectiveLine) { + std::string SpelledDirective; + llvm::raw_string_ostream OS(SpelledDirective); + OS << Prefix; + + // Make sure DirectiveRange is a char range and doesn't contain macro ids. + DirectiveRange = SM.getExpansionRange(DirectiveRange); + if (DirectiveRange.isTokenRange()) { + DirectiveRange.setEnd( + Lexer::getLocForEndOfToken(DirectiveRange.getEnd(), 0, SM, LangOpts)); + } + + auto DecompLoc = SM.getDecomposedLoc(DirectiveRange.getBegin()); + DirectiveLine = SM.getLineNumber(DecompLoc.first, DecompLoc.second); + auto TargetColumn = SM.getColumnNumber(DecompLoc.first, DecompLoc.second) - 1; + + // Pad with spaces before DirectiveRange to make sure it will be on right + // column when patched. + if (Prefix.size() <= TargetColumn) { + // There is enough space for Prefix and space before directive, use it. + // We try to squeeze the Prefix into the same line whenever we can, as + // putting onto a separate line won't work at the beginning of the file. + OS << std::string(TargetColumn - Prefix.size(), ' '); + } else { + // Prefix was longer than the space we had. We produce e.g.: + // #line N-1 + // #define \ + // X 10 + OS << "\\\n" << std::string(TargetColumn, ' '); + // Decrement because we put an additional line break before + // DirectiveRange.begin(). + --DirectiveLine; + } + OS << toSourceCode(SM, DirectiveRange.getAsRange()); + return OS.str(); +} + // Collects #define directives inside the main file. struct DirectiveCollector : public PPCallbacks { DirectiveCollector(const Preprocessor &PP, @@ -140,15 +184,12 @@ TextualDirectives.emplace_back(); TextualPPDirective &TD = TextualDirectives.back(); - auto DecompLoc = SM.getDecomposedLoc(MacroNameTok.getLocation()); - TD.DirectiveLine = SM.getLineNumber(DecompLoc.first, DecompLoc.second); - - SourceRange DefRange( - MD->getMacroInfo()->getDefinitionLoc(), - Lexer::getLocForEndOfToken(MD->getMacroInfo()->getDefinitionEndLoc(), 0, - SM, LangOpts)); - llvm::raw_string_ostream OS(TD.Text); - OS << "#define " << toSourceCode(SM, DefRange); + const auto *MI = MD->getMacroInfo(); + TD.Text = + spellDirective("#define ", + CharSourceRange::getTokenRange( + MI->getDefinitionLoc(), MI->getDefinitionEndLoc()), + LangOpts, SM, TD.DirectiveLine); } private: @@ -231,6 +272,13 @@ } llvm_unreachable("not an include directive"); } + +// Checks whether \p FileName is a valid spelling of main file. +bool isMainFile(llvm::StringRef FileName, const SourceManager &SM) { + auto FE = SM.getFileManager().getFile(FileName); + return FE && *FE == SM.getFileEntryForID(SM.getMainFileID()); +} + } // namespace PreambleData::PreambleData(const ParseInputs &Inputs, @@ -374,7 +422,7 @@ // This shouldn't coincide with any real file name. llvm::SmallString<128> PatchName; llvm::sys::path::append(PatchName, llvm::sys::path::parent_path(FileName), - "__preamble_patch__.h"); + PreamblePatchHeaderName); PP.PatchFileName = PatchName.str().str(); llvm::raw_string_ostream Patch(PP.PatchContents); @@ -428,8 +476,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(); @@ -462,5 +512,24 @@ return PP; } +SourceLocation translatePreamblePatchLocation(SourceLocation Loc, + const SourceManager &SM) { + auto DefFile = SM.getFileID(Loc); + if (auto *FE = SM.getFileEntryForID(DefFile)) { + auto IncludeLoc = SM.getIncludeLoc(DefFile); + // Preamble patch is included inside the builtin file. + if (IncludeLoc.isValid() && SM.isWrittenInBuiltinFile(IncludeLoc) && + FE->getName().endswith(PreamblePatchHeaderName)) { + auto Presumed = SM.getPresumedLoc(Loc); + // Check that line directive is pointing at main file. + if (Presumed.isValid() && Presumed.getFileID().isInvalid() && + isMainFile(Presumed.getFilename(), SM)) { + Loc = SM.translateLineCol(SM.getMainFileID(), Presumed.getLine(), + Presumed.getColumn()); + } + } + } + return Loc; +} } // namespace clangd } // namespace clang diff --git a/clang-tools-extra/clangd/SourceCode.h b/clang-tools-extra/clangd/SourceCode.h --- a/clang-tools-extra/clangd/SourceCode.h +++ b/clang-tools-extra/clangd/SourceCode.h @@ -292,6 +292,10 @@ struct DefinedMacro { llvm::StringRef Name; const MacroInfo *Info; + /// Location of the identifier that names the macro. + /// Unlike Info->Location, this translates preamble-patch locations to + /// main-file locations. + SourceLocation NameLoc; }; /// Gets the macro referenced by \p SpelledTok. It must be a spelled token /// aligned to the beginning of an identifier. diff --git a/clang-tools-extra/clangd/SourceCode.cpp b/clang-tools-extra/clangd/SourceCode.cpp --- a/clang-tools-extra/clangd/SourceCode.cpp +++ b/clang-tools-extra/clangd/SourceCode.cpp @@ -8,6 +8,7 @@ #include "SourceCode.h" #include "FuzzyMatch.h" +#include "Preamble.h" #include "Protocol.h" #include "refactor/Tweak.h" #include "support/Context.h" @@ -961,7 +962,9 @@ Loc = Loc.getLocWithOffset(-1); MacroDefinition MacroDef = PP.getMacroDefinitionAtLoc(IdentifierInfo, Loc); if (auto *MI = MacroDef.getMacroInfo()) - return DefinedMacro{IdentifierInfo->getName(), MI}; + return DefinedMacro{ + IdentifierInfo->getName(), MI, + translatePreamblePatchLocation(MI->getDefinitionLoc(), SM)}; return None; } diff --git a/clang-tools-extra/clangd/XRefs.cpp b/clang-tools-extra/clangd/XRefs.cpp --- a/clang-tools-extra/clangd/XRefs.cpp +++ b/clang-tools-extra/clangd/XRefs.cpp @@ -206,8 +206,8 @@ locateMacroReferent(const syntax::Token &TouchedIdentifier, ParsedAST &AST, llvm::StringRef MainFilePath) { if (auto M = locateMacroAt(TouchedIdentifier, AST.getPreprocessor())) { - if (auto Loc = makeLocation(AST.getASTContext(), - M->Info->getDefinitionLoc(), MainFilePath)) { + if (auto Loc = + makeLocation(AST.getASTContext(), M->NameLoc, MainFilePath)) { LocatedSymbol Macro; Macro.Name = std::string(M->Name); Macro.PreferredDeclaration = *Loc; diff --git a/clang-tools-extra/clangd/unittests/HeadersTests.cpp b/clang-tools-extra/clangd/unittests/HeadersTests.cpp --- a/clang-tools-extra/clangd/unittests/HeadersTests.cpp +++ b/clang-tools-extra/clangd/unittests/HeadersTests.cpp @@ -306,7 +306,7 @@ } TEST_F(HeadersTest, PresumedLocations) { - std::string HeaderFile = "implicit_include.h"; + std::string HeaderFile = "__preamble_patch__.h"; // Line map inclusion back to main file. std::string HeaderContents = @@ -317,7 +317,7 @@ FS.Files[HeaderFile] = HeaderContents; // Including through non-builtin file has no effects. - FS.Files[MainFile] = "#include \"implicit_include.h\"\n\n"; + FS.Files[MainFile] = "#include \"__preamble_patch__.h\"\n\n"; EXPECT_THAT(collectIncludes().MainFileIncludes, Not(Contains(Written("")))); diff --git a/clang-tools-extra/clangd/unittests/PreambleTests.cpp b/clang-tools-extra/clangd/unittests/PreambleTests.cpp --- a/clang-tools-extra/clangd/unittests/PreambleTests.cpp +++ b/clang-tools-extra/clangd/unittests/PreambleTests.cpp @@ -9,14 +9,19 @@ #include "Annotations.h" #include "Compiler.h" #include "Headers.h" +#include "Hover.h" #include "Preamble.h" +#include "SourceCode.h" #include "TestFS.h" #include "TestTU.h" +#include "XRefs.h" +#include "clang/Format/Format.h" #include "clang/Frontend/PrecompiledPreamble.h" #include "clang/Lex/PreprocessorOptions.h" #include "llvm/ADT/STLExtras.h" #include "llvm/ADT/StringExtras.h" #include "llvm/ADT/StringRef.h" +#include "llvm/Support/Error.h" #include "llvm/Support/MemoryBuffer.h" #include "llvm/Support/VirtualFileSystem.h" #include "gmock/gmock.h" @@ -28,6 +33,7 @@ using testing::Contains; using testing::Field; +using testing::Matcher; using testing::MatchesRegex; namespace clang { @@ -199,7 +205,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); } @@ -228,7 +234,8 @@ #define BAR [[BAR]])cpp", R"cpp(#line 0 ".*main.cpp" -#define BAR +#line 2 +#define BAR )cpp", }, // multiline macro @@ -238,7 +245,8 @@ [[BAR]])cpp", R"cpp(#line 0 ".*main.cpp" -#define BAR +#line 2 +#define BAR )cpp", }, // multiline macro @@ -248,7 +256,8 @@ BAR [[BAR]])cpp", R"cpp(#line 0 ".*main.cpp" -#define BAR +#line 3 +#define BAR )cpp", }, }; @@ -275,8 +284,10 @@ )cpp"); llvm::StringLiteral ExpectedPatch(R"cpp(#line 0 ".*main.cpp" -#define BAR\(X, Y\) X Y -#define BAR\(X\) X +#line 2 +#define BAR\(X, Y\) X Y +#line 3 +#define BAR\(X\) X )cpp"); EXPECT_THAT(getPreamblePatch(Baseline, Modified.code()), MatchesRegex(ExpectedPatch.str())); @@ -286,6 +297,193 @@ 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()); + ASSERT_TRUE(FoundMacro); + EXPECT_THAT(FoundMacro->Name, "FOO"); + + auto MacroLoc = FoundMacro->NameLoc; + 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()); + ASSERT_TRUE(FoundMacro); + EXPECT_THAT(FoundMacro->Name, "FOO"); + auto HI = + getHover(*AST, offsetToPosition(Modified.code(), Modified.point()), + format::getLLVMStyle(), nullptr); + ASSERT_TRUE(HI); + EXPECT_THAT(HI->Definition, testing::IsEmpty()); + } + + { + // Offset is valid, but underlying text is different. + llvm::StringLiteral Baseline = "#define FOO"; + Annotations Modified(R"cpp(#define BAR + ^FOO")cpp"); + + auto AST = createPatchedAST(Baseline, Modified.code()); + ASSERT_TRUE(AST); + + auto HI = getHover(*AST, Modified.point(), format::getLLVMStyle(), nullptr); + ASSERT_TRUE(HI); + EXPECT_THAT(HI->Definition, "#define BAR"); + } +} + +TEST(PreamblePatchTest, RefsToMacros) { + struct { + llvm::StringLiteral Baseline; + llvm::StringLiteral Modified; + } Cases[] = { + // Newly added + { + "", + R"cpp( + #define ^FOO + ^[[FOO]])cpp", + }, + // Moved around + { + "#define FOO", + R"cpp( + #define BAR + #define ^FOO + ^[[FOO]])cpp", + }, + // Ref in preamble section + { + "", + R"cpp( + #define ^FOO + #undef ^FOO)cpp", + }, + }; + + for (const auto &Case : Cases) { + Annotations Modified(Case.Modified); + auto AST = createPatchedAST("", Modified.code()); + ASSERT_TRUE(AST); + + const auto &SM = AST->getSourceManager(); + std::vector> ExpectedLocations; + for (const auto &R : Modified.ranges()) + ExpectedLocations.push_back(Field(&Location::range, R)); + + for (const auto &P : Modified.points()) { + auto *MacroTok = AST->getTokens().spelledTokenAt(SM.getComposedLoc( + SM.getMainFileID(), + llvm::cantFail(positionToOffset(Modified.code(), P)))); + ASSERT_TRUE(MacroTok); + EXPECT_THAT(findReferences(*AST, P, 0).References, + testing::ElementsAreArray(ExpectedLocations)); + } + } +} + +TEST(TranslatePreamblePatchLocation, Simple) { + auto TU = TestTU::withHeaderCode(R"cpp( + #line 3 "main.cpp" + int foo();)cpp"); + // Presumed line/col needs to be valid in the main file. + TU.Code = R"cpp(// line 1 + // line 2 + // line 3 + // line 4)cpp"; + TU.Filename = "main.cpp"; + TU.HeaderFilename = "__preamble_patch__.h"; + TU.ImplicitHeaderGuard = false; + + auto AST = TU.build(); + auto &SM = AST.getSourceManager(); + auto &ND = findDecl(AST, "foo"); + EXPECT_NE(SM.getFileID(ND.getLocation()), SM.getMainFileID()); + + auto TranslatedLoc = translatePreamblePatchLocation(ND.getLocation(), SM); + auto DecompLoc = SM.getDecomposedLoc(TranslatedLoc); + EXPECT_EQ(DecompLoc.first, SM.getMainFileID()); + EXPECT_EQ(SM.getLineNumber(DecompLoc.first, DecompLoc.second), 3U); +} } // namespace } // namespace clangd } // namespace clang