kadircet created this revision.
kadircet added a reviewer: sammccall.
Herald added a subscriber: arphaman.
Herald added a project: All.
kadircet requested review of this revision.
Herald added subscribers: cfe-commits, MaskRay, ilya-biryukov.
Herald added a project: clang-tools-extra.
Depends on D143093 <https://reviews.llvm.org/D143093>
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D143095
Files:
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
Index: clang-tools-extra/clangd/unittests/PreambleTests.cpp
===================================================================
--- clang-tools-extra/clangd/unittests/PreambleTests.cpp
+++ clang-tools-extra/clangd/unittests/PreambleTests.cpp
@@ -37,6 +37,7 @@
using testing::Contains;
using testing::ElementsAre;
+using testing::ElementsAreArray;
using testing::Field;
using testing::IsEmpty;
using testing::Matcher;
@@ -656,9 +657,8 @@
"void foo();\n#define [[FOO]] 2")
.str());
auto AST = createPatchedAST(Code.code(), NewCode.code(), AdditionalFiles);
- // FIXME: We should be pointing at the first definition of FOO in NewCode.
EXPECT_THAT(mainFileDiagRanges(*AST),
- UnorderedElementsAre(NewCode.ranges()[1]));
+ UnorderedElementsAreArray(NewCode.ranges()));
}
}
@@ -689,8 +689,9 @@
Annotations Code(BaselinePreamble);
Annotations NewCode("#define BAR\n#include [[<bar>]]\n");
auto AST = createPatchedAST(Code.code(), NewCode.code());
- // FIXME: We should point at the NewCode.
- EXPECT_THAT(mainFileDiagRanges(*AST), ElementsAre(Code.range()));
+ // FIXME: We should only have ranges from the NewCode.
+ EXPECT_THAT(mainFileDiagRanges(*AST),
+ ElementsAreArray({Code.range(), NewCode.range()}));
}
}
@@ -725,8 +726,9 @@
Annotations Code(BaselinePreamble);
Annotations NewCode("#define BAR\n#include [[<bar>]]\n#include <baz>\n");
auto AST = createPatchedAST(Code.code(), NewCode.code());
- // FIXME: We should point at the NewCode.
- EXPECT_THAT(mainFileDiagRanges(*AST), ElementsAre(Code.range()));
+ // FIXME: We should only have the range from NewCode.
+ EXPECT_THAT(mainFileDiagRanges(*AST),
+ ElementsAreArray({Code.range(), NewCode.range()}));
}
}
} // namespace
Index: clang-tools-extra/clangd/SourceCode.h
===================================================================
--- clang-tools-extra/clangd/SourceCode.h
+++ 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
Index: clang-tools-extra/clangd/SourceCode.cpp
===================================================================
--- clang-tools-extra/clangd/SourceCode.cpp
+++ clang-tools-extra/clangd/SourceCode.cpp
@@ -1219,5 +1219,30 @@
return SM.getBufferData(FID).startswith(ProtoHeaderComment);
}
+SourceLocation translatePreamblePatchLocation(SourceLocation Loc,
+ const SourceManager &SM) {
+ // Checks whether \p FileName is a valid spelling of main file.
+ auto IsMainFile = [](llvm::StringRef FileName, const SourceManager &SM) {
+ auto FE = SM.getFileManager().getFile(FileName);
+ return FE && *FE == SM.getFileEntryForID(SM.getMainFileID());
+ };
+
+ 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
Index: clang-tools-extra/clangd/Preamble.h
===================================================================
--- clang-tools-extra/clangd/Preamble.h
+++ 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
Index: clang-tools-extra/clangd/Preamble.cpp
===================================================================
--- clang-tools-extra/clangd/Preamble.cpp
+++ 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;
@@ -768,26 +761,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.Mode != Config::DiagnosticsMode::Strict;
Index: clang-tools-extra/clangd/Diagnostics.cpp
===================================================================
--- clang-tools-extra/clangd/Diagnostics.cpp
+++ clang-tools-extra/clangd/Diagnostics.cpp
@@ -98,17 +98,23 @@
// LSP needs a single range.
Range diagnosticRange(const clang::Diagnostic &D, const LangOptions &L) {
auto &M = D.getSourceManager();
+ auto PatchedRange = [&M](CharSourceRange &R) {
+ // FIXME: Should we handle all presumed locations?
+ 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 +123,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 +219,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 +712,14 @@
auto FillDiagBase = [&](DiagBase &D) {
fillNonLocationData(DiagLevel, Info, D);
- D.InsideMainFile = isInsideMainFile(Info);
+ auto PLoc = translatePreamblePatchLocation(Info.getLocation(), SM);
+ D.InsideMainFile = isInsideMainFile(PLoc, 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;
};
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
