kadircet updated this revision to Diff 493382.
kadircet added a comment.
Well, turns out presumed location patching was not handled in clangd at all for
diagnostics. So add that to translate ranges, at least when we have ranges or
locations that point to preamble patch.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D142892/new/
https://reviews.llvm.org/D142892
Files:
clang-tools-extra/clangd/Diagnostics.cpp
clang-tools-extra/clangd/ParsedAST.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
@@ -8,9 +8,13 @@
#include "Annotations.h"
#include "Compiler.h"
+#include "Config.h"
+#include "Diagnostics.h"
#include "Headers.h"
#include "Hover.h"
+#include "ParsedAST.h"
#include "Preamble.h"
+#include "Protocol.h"
#include "SourceCode.h"
#include "TestFS.h"
#include "TestTU.h"
@@ -18,10 +22,12 @@
#include "clang/Format/Format.h"
#include "clang/Frontend/FrontendActions.h"
#include "clang/Frontend/PrecompiledPreamble.h"
+#include "llvm/ADT/StringMap.h"
#include "llvm/ADT/StringRef.h"
#include "llvm/Support/Error.h"
#include "llvm/Support/MemoryBuffer.h"
#include "llvm/Support/VirtualFileSystem.h"
+#include "llvm/Testing/Support/SupportHelpers.h"
#include "gmock/gmock.h"
#include "gtest/gtest.h"
#include <memory>
@@ -30,9 +36,14 @@
#include <vector>
using testing::Contains;
+using testing::ElementsAre;
using testing::Field;
+using testing::IsEmpty;
using testing::Matcher;
using testing::MatchesRegex;
+using testing::Not;
+using testing::UnorderedElementsAre;
+using testing::UnorderedElementsAreArray;
namespace clang {
namespace clangd {
@@ -190,9 +201,12 @@
Field(&Inclusion::Resolved, testPath("a.h")))));
}
-std::optional<ParsedAST> createPatchedAST(llvm::StringRef Baseline,
- llvm::StringRef Modified) {
- auto BaselinePreamble = TestTU::withCode(Baseline).preamble();
+std::optional<ParsedAST>
+createPatchedAST(llvm::StringRef Baseline, llvm::StringRef Modified,
+ llvm::StringMap<std::string> AdditionalFiles = {}) {
+ auto PreambleTU = TestTU::withCode(Baseline);
+ PreambleTU.AdditionalFiles = AdditionalFiles;
+ auto BaselinePreamble = PreambleTU.preamble();
if (!BaselinePreamble) {
ADD_FAILURE() << "Failed to build baseline preamble";
return std::nullopt;
@@ -201,6 +215,7 @@
IgnoreDiagnostics Diags;
MockFS FS;
auto TU = TestTU::withCode(Modified);
+ TU.AdditionalFiles = std::move(AdditionalFiles);
auto CI = buildCompilerInvocation(TU.inputs(FS), Diags);
if (!CI) {
ADD_FAILURE() << "Failed to build compiler invocation";
@@ -236,6 +251,7 @@
#define BAR
[[BAR]])cpp",
R"cpp(#line 0 ".*main.cpp"
+#undef BAR
#line 2
#define BAR
)cpp",
@@ -247,6 +263,7 @@
[[BAR]])cpp",
R"cpp(#line 0 ".*main.cpp"
+#undef BAR
#line 2
#define BAR
)cpp",
@@ -258,6 +275,7 @@
BAR
[[BAR]])cpp",
R"cpp(#line 0 ".*main.cpp"
+#undef BAR
#line 3
#define BAR
)cpp",
@@ -290,8 +308,10 @@
)cpp");
llvm::StringLiteral ExpectedPatch(R"cpp(#line 0 ".*main.cpp"
+#undef BAR
#line 2
#define BAR\(X, Y\) X Y
+#undef BAR
#line 3
#define BAR\(X\) X
)cpp");
@@ -565,6 +585,141 @@
TU.inputs(FS), *BaselinePreamble);
EXPECT_TRUE(PP.text().empty());
}
+
+std::vector<clangd::Range> mainFileDiagRanges(const ParsedAST &AST) {
+ std::vector<clangd::Range> Result;
+ auto AddRangeIfInMainfile = [&Result](const DiagBase &DB) {
+ if (DB.InsideMainFile)
+ Result.emplace_back(DB.Range);
+ };
+ for (auto &D : *AST.getDiagnostics()) {
+ AddRangeIfInMainfile(D);
+ for (auto &N : D.Notes)
+ AddRangeIfInMainfile(N);
+ }
+ return Result;
+}
+
+TEST(PreamblePatch, DiagnosticsFromMainASTAreInRightPlace) {
+ Config Cfg;
+ Cfg.Diagnostics.Mode = Config::DiagnosticsMode::Fast;
+ WithContextValue WithCfg(Config::Key, std::move(Cfg));
+
+ llvm::StringLiteral BaselinePreamble = "#define FOO\n";
+ {
+ // Check with removals from preamble.
+ Annotations Code("[[x]];/* error-ok */");
+ auto AST = createPatchedAST(BaselinePreamble, Code.code());
+ EXPECT_THAT(mainFileDiagRanges(*AST), ElementsAre(Code.range()));
+ }
+ {
+ // Check with additions to preamble.
+ Annotations Code(
+ (BaselinePreamble + "#define BAR\n[[x]];/* error-ok */").str());
+ auto AST = createPatchedAST(BaselinePreamble, Code.code());
+ EXPECT_THAT(mainFileDiagRanges(*AST), ElementsAre(Code.range()));
+ }
+}
+
+TEST(PreamblePatch, DiagnosticsToPreamble) {
+ Config Cfg;
+ Cfg.Diagnostics.Mode = Config::DiagnosticsMode::Fast;
+ Cfg.Diagnostics.UnusedIncludes = Config::UnusedIncludesPolicy::Strict;
+ WithContextValue WithCfg(Config::Key, std::move(Cfg));
+
+ llvm::StringMap<std::string> AdditionalFiles;
+ AdditionalFiles["foo.h"] = "#pragma once";
+ AdditionalFiles["bar.h"] = "#pragma once";
+ llvm::StringLiteral BaselinePreamble("#define FOO 1\n[[#include \"foo.h\"]]");
+ {
+ // Check with removals from preamble.
+ Annotations NewCode("[[# include \"foo.h\"]]");
+ auto AST = createPatchedAST(Annotations(BaselinePreamble).code(),
+ NewCode.code(), AdditionalFiles);
+ EXPECT_THAT(mainFileDiagRanges(*AST), ElementsAre(NewCode.range()));
+ }
+ {
+ // Check with additions to preamble.
+ Annotations NewCode(("[[#include \"bar.h\"]]\n" + BaselinePreamble).str());
+ auto AST = createPatchedAST(Annotations(BaselinePreamble).code(),
+ NewCode.code(), AdditionalFiles);
+ EXPECT_THAT(mainFileDiagRanges(*AST),
+ UnorderedElementsAreArray(NewCode.ranges()));
+ }
+ {
+ llvm::StringLiteral BaselinePreamble = "#define [[FOO]] 1\n";
+ // Check ranges for notes.
+ Annotations NewCode(("#define BARXYZ 1\n" + BaselinePreamble +
+ "void foo();\n#define [[FOO]] 2")
+ .str());
+ auto AST = createPatchedAST(Annotations(BaselinePreamble).code(),
+ NewCode.code(), AdditionalFiles);
+ EXPECT_THAT(mainFileDiagRanges(*AST),
+ UnorderedElementsAreArray(NewCode.ranges()));
+ }
+}
+
+TEST(PreamblePatch, TranslatesIncludeDiagnosticsInPreamble) {
+ Config Cfg;
+ Cfg.Diagnostics.Mode = Config::DiagnosticsMode::Fast;
+ WithContextValue WithCfg(Config::Key, std::move(Cfg));
+
+ llvm::StringLiteral BaselinePreamble("#include [[<foo>]]\n");
+ {
+ // Check with additions to preamble.
+ Annotations Code(BaselinePreamble);
+ Annotations NewCode(("#define BAR\n" + BaselinePreamble).str());
+ auto AST = createPatchedAST(Code.code(), NewCode.code());
+ EXPECT_THAT(mainFileDiagRanges(*AST), ElementsAre(NewCode.range()));
+ }
+ {
+ // Check with removals from preamble.
+ Annotations Code(("#define BAR\n" + BaselinePreamble).str());
+ Annotations NewCode(BaselinePreamble);
+ auto AST = createPatchedAST(Code.code(), NewCode.code());
+ EXPECT_THAT(mainFileDiagRanges(*AST), ElementsAre(NewCode.range()));
+ }
+ {
+ // Drop line with diags.
+ Annotations Code(BaselinePreamble);
+ Annotations NewCode("#define BAR\n#include [[<bar>]]\n");
+ auto AST = createPatchedAST(Code.code(), NewCode.code());
+ EXPECT_THAT(mainFileDiagRanges(*AST), ElementsAre(NewCode.range()));
+ }
+}
+
+TEST(PreamblePatch, TranslatesNonIncludeDiagnosticsInPreamble) {
+ Config Cfg;
+ Cfg.Diagnostics.Mode = Config::DiagnosticsMode::Fast;
+ WithContextValue WithCfg(Config::Key, std::move(Cfg));
+
+ llvm::StringLiteral BaselinePreamble("#define [[__LINE__]]\n");
+ ASSERT_THAT(TestTU::withCode(Annotations(BaselinePreamble).code())
+ .build()
+ .getDiagnostics(),
+ Not(llvm::ValueIs(IsEmpty())));
+ {
+ // Check with additions to preamble.
+ Annotations Code(BaselinePreamble);
+ Annotations NewCode(("#define BAR\n" + BaselinePreamble).str());
+ auto AST = createPatchedAST(Code.code(), NewCode.code());
+ EXPECT_THAT(mainFileDiagRanges(*AST), ElementsAre(NewCode.range()));
+ }
+ {
+ // Check with removals from preamble.
+ Annotations Code(("#define BAR\n" + BaselinePreamble).str());
+ Annotations NewCode(BaselinePreamble);
+ auto AST = createPatchedAST(Code.code(), NewCode.code());
+ EXPECT_THAT(mainFileDiagRanges(*AST), ElementsAre(NewCode.range()));
+ }
+ {
+ // Drop line with diags.
+ Annotations Code(BaselinePreamble);
+ Annotations NewCode("#define BAR\n#include [[<bar>]]\n#include <baz>\n");
+ auto AST = createPatchedAST(Code.code(), NewCode.code());
+ EXPECT_THAT(mainFileDiagRanges(*AST), ElementsAre(NewCode.range()));
+ }
+}
} // namespace
} // namespace clangd
} // namespace clang
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
@@ -155,7 +155,12 @@
llvm::StringRef text() const { return PatchContents; }
/// Whether diagnostics generated using this patch are trustable.
- bool preserveDiagnostics() const { return PatchContents.empty(); }
+ bool preserveDiagnostics() const;
+
+ /// Returns diag locations for Modified contents, only contains diags attached
+ /// to an #include or #define directive.
+ std::vector<Diag> patchedDiags() const;
+ static constexpr llvm::StringLiteral HeaderName = "__preamble_patch__.h";
private:
static PreamblePatch create(llvm::StringRef FileName,
@@ -166,17 +171,14 @@
PreamblePatch() = default;
std::string PatchContents;
std::string PatchFileName;
- /// Includes that are present in both \p Baseline and \p Modified. Used for
- /// patching includes of baseline preamble.
+ // Includes that are present in both \p Baseline and \p Modified. Used for
+ // patching includes of baseline preamble.
std::vector<Inclusion> PreambleIncludes;
+ // Diags that were attached to an #include or a #define directive.
+ std::vector<Diag> PatchedDiags;
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
@@ -9,6 +9,7 @@
#include "Preamble.h"
#include "Compiler.h"
#include "Config.h"
+#include "Diagnostics.h"
#include "Headers.h"
#include "SourceCode.h"
#include "clang-include-cleaner/Record.h"
@@ -35,6 +36,8 @@
#include "llvm/ADT/IntrusiveRefCntPtr.h"
#include "llvm/ADT/STLExtras.h"
#include "llvm/ADT/SmallString.h"
+#include "llvm/ADT/SmallVector.h"
+#include "llvm/ADT/StringMap.h"
#include "llvm/ADT/StringRef.h"
#include "llvm/Support/Error.h"
#include "llvm/Support/ErrorHandling.h"
@@ -43,7 +46,7 @@
#include "llvm/Support/Path.h"
#include "llvm/Support/VirtualFileSystem.h"
#include "llvm/Support/raw_ostream.h"
-#include <iterator>
+#include <map>
#include <memory>
#include <string>
#include <system_error>
@@ -53,7 +56,6 @@
namespace clang {
namespace clangd {
namespace {
-constexpr llvm::StringLiteral PreamblePatchHeaderName = "__preamble_patch__.h";
bool compileCommandsAreEqual(const tooling::CompileCommand &LHS,
const tooling::CompileCommand &RHS) {
@@ -213,6 +215,9 @@
// Full text that's representing the directive, including the `#`.
std::string Text;
unsigned Offset;
+ tok::PPKeywordKind Directive;
+ // Name of the macro being defined in the case of a #define directive.
+ std::string MacroName;
bool operator==(const TextualPPDirective &RHS) const {
return std::tie(DirectiveLine, Offset, Text) ==
@@ -283,6 +288,8 @@
return;
TextualDirectives.emplace_back();
TextualPPDirective &TD = TextualDirectives.back();
+ TD.Directive = tok::pp_define;
+ TD.MacroName = MacroNameTok.getIdentifierInfo()->getName().str();
const auto *MI = MD->getMacroInfo();
TD.Text =
@@ -376,12 +383,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
@@ -467,6 +468,19 @@
WallTimer Timer;
};
+// Represents an include as spelled in the source code, without taking resolved
+// path or location into account.
+struct IncludeKey {
+ tok::PPKeywordKind Directive;
+ llvm::StringRef Written;
+
+ bool operator<(const IncludeKey &RHS) const {
+ return std::tie(Directive, Written) < std::tie(RHS.Directive, RHS.Written);
+ }
+ bool operator==(const IncludeKey &RHS) const {
+ return std::tie(Directive, Written) == std::tie(RHS.Directive, RHS.Written);
+ }
+};
} // namespace
std::shared_ptr<const PreambleData>
@@ -614,6 +628,70 @@
}
}
+// Checks if all pointers in \p D are for the same line of the main file.
+static bool diagReferencesMultipleLines(const Diag &D) {
+ int Line = D.Range.start.line;
+ if (D.Range.end.line != Line)
+ return true;
+ bool NotePointsToOutside = llvm::any_of(D.Notes, [&](const Note &N) {
+ return N.File == D.File &&
+ (N.Range.start.line != Line || N.Range.end.line != Line);
+ });
+ if (NotePointsToOutside)
+ return true;
+ bool FixPointsToOutside = llvm::any_of(D.Fixes, [&](const Fix &F) {
+ for (auto &E : F.Edits)
+ if (E.range.start.line != Line || E.range.end.line != Line)
+ return true;
+ return false;
+ });
+ if (FixPointsToOutside)
+ return true;
+ return false;
+}
+
+// Move all references in \p D to \p NewLine. This assumes none of the
+// references to the main file are multi-line.
+static Diag translateDiag(const Diag &D, int NewLine) {
+ assert(!diagReferencesMultipleLines(D));
+ Diag Translated = D;
+ Translated.Range.start.line = Translated.Range.end.line = NewLine;
+ for (auto &N : Translated.Notes) {
+ if (N.File != D.File)
+ continue;
+ N.Range.start.line = N.Range.end.line = NewLine;
+ }
+ for (auto &F : Translated.Fixes) {
+ for (auto &E : F.Edits)
+ E.range.start.line = E.range.end.line = NewLine;
+ }
+ return Translated;
+}
+
+static llvm::DenseMap<int, llvm::SmallVector<const Diag *>>
+mapDiagsToLines(llvm::ArrayRef<Diag> Diags) {
+ llvm::DenseMap<int, llvm::SmallVector<const Diag *>> LineToDiags;
+ for (auto &D : Diags) {
+ if (diagReferencesMultipleLines(D))
+ continue;
+ LineToDiags[D.Range.start.line].emplace_back(&D);
+ }
+ return LineToDiags;
+}
+
+static std::map<IncludeKey, const Inclusion *>
+getExistingIncludes(const ScannedPreamble &BaselineScan,
+ llvm::ArrayRef<Inclusion> MFI) {
+ std::map<IncludeKey, const Inclusion *> ExistingIncludes;
+ // There might be includes coming from disabled regions, record these for
+ // exclusion. note that we don't have resolved paths for those.
+ for (const auto &Inc : BaselineScan.Includes)
+ ExistingIncludes[{Inc.Directive, Inc.Written}] = nullptr;
+ for (const auto &Inc : MFI)
+ ExistingIncludes[{Inc.Directive, Inc.Written}] = &Inc;
+ return ExistingIncludes;
+}
+
PreamblePatch PreamblePatch::create(llvm::StringRef FileName,
const ParseInputs &Modified,
const PreambleData &Baseline,
@@ -655,7 +733,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;
@@ -667,26 +745,37 @@
escapeBackslashAndQuotes(FileName, Patch);
Patch << "\"\n";
+ // List of diagnostics associated to a particular line in the baseline.
+ auto LineToDiags = mapDiagsToLines(Baseline.Diags);
+ auto MoveDiagsBetweenLines = [&LineToDiags, &PP](int OldLine, int NewLine) {
+ auto DiagsForInclude = LineToDiags.find(OldLine);
+ if (DiagsForInclude == LineToDiags.end())
+ return;
+ for (auto *D : DiagsForInclude->second)
+ PP.PatchedDiags.emplace_back(translateDiag(*D, NewLine));
+ };
+ // Map from an include to its line in the Modified contents.
+ std::map<IncludeKey, int> IncludeToPatchedLine;
if (IncludesChanged && PatchType == PatchType::All) {
// We are only interested in newly added includes, record the ones in
// Baseline for exclusion.
- llvm::DenseMap<std::pair<tok::PPKeywordKind, llvm::StringRef>,
- /*Resolved=*/llvm::StringRef>
- ExistingIncludes;
- for (const auto &Inc : Baseline.Includes.MainFileIncludes)
- ExistingIncludes[{Inc.Directive, Inc.Written}] = Inc.Resolved;
- // There might be includes coming from disabled regions, record these for
- // exclusion too. note that we don't have resolved paths for those.
- for (const auto &Inc : BaselineScan->Includes)
- ExistingIncludes.try_emplace({Inc.Directive, Inc.Written});
+ auto ExistingIncludes =
+ getExistingIncludes(*BaselineScan, Baseline.Includes.MainFileIncludes);
// Calculate extra includes that needs to be inserted.
for (auto &Inc : ModifiedScan->Includes) {
auto It = ExistingIncludes.find({Inc.Directive, Inc.Written});
// Include already present in the baseline preamble. Set resolved path and
// put into preamble includes.
if (It != ExistingIncludes.end()) {
- Inc.Resolved = It->second.str();
+ if (It->second) {
+ // Copy everything from existing include, apart from the location.
+ Inc.Resolved = It->second->Resolved;
+ Inc.HeaderID = It->second->HeaderID;
+ Inc.BehindPragmaKeep = It->second->BehindPragmaKeep;
+ Inc.FileKind = It->second->FileKind;
+ }
PP.PreambleIncludes.push_back(Inc);
+ IncludeToPatchedLine[It->first] = Inc.HashLine;
continue;
}
// Include is new in the modified preamble. Inject it into the patch and
@@ -696,8 +785,29 @@
Patch << llvm::formatv(
"#{0} {1}\n", spellingForIncDirective(Inc.Directive), Inc.Written);
}
+ } else {
+ // Copy include information from baseline preamble if nothing has changed.
+ PP.PreambleIncludes = Baseline.Includes.MainFileIncludes;
}
+ // Patch locations for diagnostics on baseline includes.
+ for (const auto &Inc : Baseline.Includes.MainFileIncludes) {
+ // Use existing locations if includes haven't changed.
+ size_t NewLine = Inc.HashLine;
+ if (IncludesChanged) {
+ auto PatchedLine =
+ IncludeToPatchedLine.find({Inc.Directive, Inc.Written});
+ // Ignore diags for deleted includes.
+ if (PatchedLine == IncludeToPatchedLine.end())
+ continue;
+ NewLine = PatchedLine->second;
+ }
+ MoveDiagsBetweenLines(Inc.HashLine, NewLine);
+ }
+
+ // Maps a directive from its spelling to its location in the Modified
+ // contents.
+ llvm::StringMap<int> DirectiveToPatchedLine;
if (DirectivesChanged) {
// We need to patch all the directives, since they are order dependent. e.g:
// #define BAR(X) NEW(X) // Newly introduced in Modified
@@ -711,10 +821,29 @@
// reduce complexity. The former might cause problems because scanning is
// imprecise and might pick directives from disabled regions.
for (const auto &TD : ModifiedScan->TextualDirectives) {
+ // Introduce an #undef directive before #defines to suppress any
+ // re-definition warnings.
+ if (TD.Directive == tok::pp_define)
+ Patch << "#undef " << TD.MacroName << '\n';
Patch << "#line " << TD.DirectiveLine << '\n';
Patch << TD.Text << '\n';
+ DirectiveToPatchedLine[TD.Text] = TD.DirectiveLine;
}
+ } else {
+ // Take existing directives as-is, if they're the same.
+ for (const auto &TD : BaselineScan->TextualDirectives)
+ DirectiveToPatchedLine[TD.Text] = TD.DirectiveLine;
+ }
+
+ // Patch locations for diagnositcs on baseline macros.
+ for (const auto &TD : BaselineScan->TextualDirectives) {
+ // Textual directive lines are 1-based.
+ auto NewLine = DirectiveToPatchedLine.find(TD.Text);
+ if (NewLine == DirectiveToPatchedLine.end())
+ continue;
+ MoveDiagsBetweenLines(TD.DirectiveLine - 1, NewLine->second - 1);
}
+
dlog("Created preamble patch: {0}", Patch.str());
Patch.flush();
return PP;
@@ -756,28 +885,14 @@
PreamblePatch PP;
PP.PreambleIncludes = Preamble.Includes.MainFileIncludes;
PP.ModifiedBounds = Preamble.Preamble.getBounds();
+ PP.PatchedDiags = Preamble.Diags;
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;
}
-
+std::vector<Diag> PreamblePatch::patchedDiags() const { return PatchedDiags; }
} // namespace clangd
} // namespace clang
Index: clang-tools-extra/clangd/ParsedAST.cpp
===================================================================
--- clang-tools-extra/clangd/ParsedAST.cpp
+++ clang-tools-extra/clangd/ParsedAST.cpp
@@ -674,9 +674,10 @@
if (PreserveDiags) {
Diags = CompilerInvocationDiags;
// Add diagnostics from the preamble, if any.
- if (Preamble)
- Diags->insert(Diags->end(), Preamble->Diags.begin(),
- Preamble->Diags.end());
+ if (Preamble) {
+ auto PDiags = Patch->patchedDiags();
+ Diags->insert(Diags->end(), PDiags.begin(), PDiags.end());
+ }
// Finally, add diagnostics coming from the AST.
{
std::vector<Diag> D = ASTDiags.take(&*CTContext);
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
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
