diff --git a/clang-tools-extra/clangd/Diagnostics.cpp b/clang-tools-extra/clangd/Diagnostics.cpp --- a/clang-tools-extra/clangd/Diagnostics.cpp +++ b/clang-tools-extra/clangd/Diagnostics.cpp @@ -98,17 +98,22 @@ // LSP needs a single range. Range diagnosticRange(const clang::Diagnostic &D, const LangOptions &L) { auto &M = D.getSourceManager(); + auto PatchedRange = [&M](CharSourceRange &R) { + R.setBegin(translatePreamblePatchLocation(R.getBegin(), M)); + R.setEnd(translatePreamblePatchLocation(R.getEnd(), M)); + return R; + }; auto Loc = M.getFileLoc(D.getLocation()); for (const auto &CR : D.getRanges()) { auto R = Lexer::makeFileCharRange(CR, M, L); if (locationInRange(Loc, R, M)) - return halfOpenToRange(M, R); + return halfOpenToRange(M, PatchedRange(R)); } // The range may be given as a fixit hint instead. for (const auto &F : D.getFixItHints()) { auto R = Lexer::makeFileCharRange(F.RemoveRange, M, L); if (locationInRange(Loc, R, M)) - return halfOpenToRange(M, R); + return halfOpenToRange(M, PatchedRange(R)); } // If the token at the location is not a comment, we use the token. // If we can't get the token at the location, fall back to using the location @@ -117,7 +122,7 @@ if (!Lexer::getRawToken(Loc, Tok, M, L, true) && Tok.isNot(tok::comment)) { R = CharSourceRange::getTokenRange(Tok.getLocation(), Tok.getEndLoc()); } - return halfOpenToRange(M, R); + return halfOpenToRange(M, PatchedRange(R)); } // Try to find a location in the main-file to report the diagnostic D. @@ -213,13 +218,6 @@ return true; } -bool isInsideMainFile(const clang::Diagnostic &D) { - if (!D.hasSourceManager()) - return false; - - return clangd::isInsideMainFile(D.getLocation(), D.getSourceManager()); -} - bool isNote(DiagnosticsEngine::Level L) { return L == DiagnosticsEngine::Note || L == DiagnosticsEngine::Remark; } @@ -713,11 +711,15 @@ auto FillDiagBase = [&](DiagBase &D) { fillNonLocationData(DiagLevel, Info, D); - D.InsideMainFile = isInsideMainFile(Info); + SourceLocation PatchLoc = + translatePreamblePatchLocation(Info.getLocation(), SM); + D.InsideMainFile = isInsideMainFile(PatchLoc, SM); D.Range = diagnosticRange(Info, *LangOpts); - D.File = std::string(SM.getFilename(Info.getLocation())); - D.AbsFile = getCanonicalPath( - SM.getFileEntryForID(SM.getFileID(Info.getLocation())), SM); + auto FID = SM.getFileID(Info.getLocation()); + if (auto *FE = SM.getFileEntryForID(FID)) { + D.File = FE->getName().str(); + D.AbsFile = getCanonicalPath(FE, SM); + } D.ID = Info.getID(); return D; }; 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 @@ -157,6 +157,8 @@ /// Whether diagnostics generated using this patch are trustable. bool preserveDiagnostics() const; + static constexpr llvm::StringLiteral HeaderName = "__preamble_patch__.h"; + private: static PreamblePatch create(llvm::StringRef FileName, const ParseInputs &Modified, @@ -172,11 +174,6 @@ PreambleBounds ModifiedBounds = {0, false}; }; -/// 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 @@ -53,7 +53,6 @@ namespace clang { namespace clangd { namespace { -constexpr llvm::StringLiteral PreamblePatchHeaderName = "__preamble_patch__.h"; bool compileCommandsAreEqual(const tooling::CompileCommand &LHS, const tooling::CompileCommand &RHS) { @@ -381,12 +380,6 @@ 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()); -} - // Accumulating wall time timer. Similar to llvm::Timer, but much cheaper, // it only tracks wall time. // Since this is a generic timer, We may want to move this to support/ if we @@ -660,7 +653,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), - PreamblePatchHeaderName); + PreamblePatch::HeaderName); PP.PatchFileName = PatchName.str().str(); PP.ModifiedBounds = ModifiedScan->Bounds; @@ -781,26 +774,6 @@ return PP; } -SourceLocation translatePreamblePatchLocation(SourceLocation Loc, - const SourceManager &SM) { - auto DefFile = SM.getFileID(Loc); - if (auto FE = SM.getFileEntryRefForID(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; -} - bool PreamblePatch::preserveDiagnostics() const { return PatchContents.empty() || Config::current().Diagnostics.AllowStalePreamble; 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 @@ -333,6 +333,10 @@ (isUppercase(Name[1]) || Name[1] == '_'); } +/// 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 #endif 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 @@ -796,6 +796,12 @@ return Results; } +// 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 std::vector visibleNamespaces(llvm::StringRef Code, @@ -1219,5 +1225,24 @@ return SM.getBufferData(FID).startswith(ProtoHeaderComment); } +SourceLocation translatePreamblePatchLocation(SourceLocation Loc, + const SourceManager &SM) { + auto DefFile = SM.getFileID(Loc); + if (auto FE = SM.getFileEntryRefForID(DefFile)) { + auto IncludeLoc = SM.getIncludeLoc(DefFile); + // Preamble patch is included inside the builtin file. + if (IncludeLoc.isValid() && SM.isWrittenInBuiltinFile(IncludeLoc) && + FE->getName().endswith(PreamblePatch::HeaderName)) { + 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/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 @@ -41,6 +41,7 @@ using testing::AllOf; using testing::Contains; using testing::ElementsAre; +using testing::ElementsAreArray; using testing::Eq; using testing::Field; using testing::HasSubstr; @@ -706,10 +707,8 @@ auto AST = createPatchedAST(Code.code(), NewCode.code(), AdditionalFiles); EXPECT_THAT( *AST->getDiagnostics(), - ElementsAre(AllOf( - Diag(NewCode.range("foo2"), "-Wmacro-redefined"), - // FIXME: This should be translated into main file. - withNote(Field(&Note::File, HasSubstr("_preamble_patch_")))))); + ElementsAre(AllOf(Diag(NewCode.range("foo2"), "-Wmacro-redefined"), + withNote(Diag(NewCode.range("foo1")))))); } }