[clang-tools-extra] 909cd1f - [clangd] Respect preamble-patch when handling diags
Kadir Cetinkaya via cfe-commits
cfe-commits at lists.llvm.org
Wed Feb 22 06:54:43 PST 2023
Author: Kadir Cetinkaya
Date: 2023-02-22T15:54:15+01:00
New Revision: 909cd1f9a8934033a803659da8c54c87013941ef
URL: https://github.com/llvm/llvm-project/commit/909cd1f9a8934033a803659da8c54c87013941ef
DIFF: https://github.com/llvm/llvm-project/commit/909cd1f9a8934033a803659da8c54c87013941ef.diff
LOG: [clangd] Respect preamble-patch when handling diags
Depends on D143093
Differential Revision: https://reviews.llvm.org/D143095
Added:
Modified:
clang-tools-extra/clangd/Diagnostics.cpp
clang-tools-extra/clangd/Preamble.cpp
clang-tools-extra/clangd/Preamble.h
clang-tools-extra/clangd/SourceCode.cpp
clang-tools-extra/clangd/SourceCode.h
clang-tools-extra/clangd/unittests/PreambleTests.cpp
Removed:
################################################################################
diff --git a/clang-tools-extra/clangd/Diagnostics.cpp b/clang-tools-extra/clangd/Diagnostics.cpp
index 652e9e13b6e2..26b0047951b5 100644
--- a/clang-tools-extra/clangd/Diagnostics.cpp
+++ b/clang-tools-extra/clangd/Diagnostics.cpp
@@ -98,17 +98,22 @@ bool locationInRange(SourceLocation L, CharSourceRange R,
// 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 @@ Range diagnosticRange(const clang::Diagnostic &D, const LangOptions &L) {
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 @@ bool tryMoveToMainFile(Diag &D, FullSourceLoc DiagLoc) {
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 @@ void StoreDiags::HandleDiagnostic(DiagnosticsEngine::Level DiagLevel,
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.cpp b/clang-tools-extra/clangd/Preamble.cpp
index d01601e5a037..6a7efcd255d2 100644
--- 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 @@ const char *spellingForIncDirective(tok::PPKeywordKind IncludeDirective) {
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 @@ PreamblePatch PreamblePatch::create(llvm::StringRef FileName,
// 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 @@ PreamblePatch PreamblePatch::unmodified(const PreambleData &Preamble) {
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/Preamble.h b/clang-tools-extra/clangd/Preamble.h
index 438a4561cac1..8bc65331f3e0 100644
--- a/clang-tools-extra/clangd/Preamble.h
+++ b/clang-tools-extra/clangd/Preamble.h
@@ -157,6 +157,8 @@ class PreamblePatch {
/// 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 @@ class PreamblePatch {
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/SourceCode.cpp b/clang-tools-extra/clangd/SourceCode.cpp
index d0140a7e0d01..044562ec1b59 100644
--- a/clang-tools-extra/clangd/SourceCode.cpp
+++ b/clang-tools-extra/clangd/SourceCode.cpp
@@ -796,6 +796,12 @@ llvm::SmallVector<llvm::StringRef> ancestorNamespaces(llvm::StringRef NS) {
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<std::string> visibleNamespaces(llvm::StringRef Code,
@@ -1219,5 +1225,24 @@ bool isProtoFile(SourceLocation Loc, const SourceManager &SM) {
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/SourceCode.h b/clang-tools-extra/clangd/SourceCode.h
index 7527fd944ea9..8643fab09656 100644
--- a/clang-tools-extra/clangd/SourceCode.h
+++ b/clang-tools-extra/clangd/SourceCode.h
@@ -333,6 +333,10 @@ inline bool isReservedName(llvm::StringRef Name) {
(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/unittests/PreambleTests.cpp b/clang-tools-extra/clangd/unittests/PreambleTests.cpp
index 68de063f7168..2425e430ae6e 100644
--- 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;
@@ -705,10 +706,8 @@ void foo();
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"))))));
}
}
More information about the cfe-commits
mailing list