diff options
author | Kadir Cetinkaya <kadircet@google.com> | 2023-04-12 18:40:43 +0200 |
---|---|---|
committer | Kadir Cetinkaya <kadircet@google.com> | 2023-04-12 21:03:21 +0200 |
commit | 144562e678d932dc2685f8a83aeb2229bc96d71a (patch) | |
tree | 218f61e0afbeec986a504f8f916671f19178a1f1 /clang-tools-extra/clangd | |
parent | 9af92455f36f08c111625dcd4951f91003bacee2 (diff) | |
download | llvm-144562e678d932dc2685f8a83aeb2229bc96d71a.tar.gz |
[clangd] Treat preamble patch as main file for include-cleaner analysis
Since we redefine all macros in preamble-patch, and it's parsed after
consuming the preamble macros, we can get false missing-include diagnostics
while a fresh preamble is being rebuilt.
This patch makes sure preamble-patch is treated same as main file for
include-cleaner purposes.
Differential Revision: https://reviews.llvm.org/D148143
Diffstat (limited to 'clang-tools-extra/clangd')
-rw-r--r-- | clang-tools-extra/clangd/IncludeCleaner.cpp | 8 | ||||
-rw-r--r-- | clang-tools-extra/clangd/Preamble.cpp | 22 | ||||
-rw-r--r-- | clang-tools-extra/clangd/Preamble.h | 8 | ||||
-rw-r--r-- | clang-tools-extra/clangd/unittests/PreambleTests.cpp | 24 |
4 files changed, 54 insertions, 8 deletions
diff --git a/clang-tools-extra/clangd/IncludeCleaner.cpp b/clang-tools-extra/clangd/IncludeCleaner.cpp index ee645efa6e6e..e645b1cce6a0 100644 --- a/clang-tools-extra/clangd/IncludeCleaner.cpp +++ b/clang-tools-extra/clangd/IncludeCleaner.cpp @@ -11,6 +11,7 @@ #include "Diagnostics.h" #include "Headers.h" #include "ParsedAST.h" +#include "Preamble.h" #include "Protocol.h" #include "SourceCode.h" #include "URI.h" @@ -112,12 +113,12 @@ static bool mayConsiderUnused(const Inclusion &Inc, ParsedAST &AST, return false; // Check if main file is the public interface for a private header. If so we // shouldn't diagnose it as unused. - if(auto PHeader = PI->getPublic(*FE); !PHeader.empty()) { + if (auto PHeader = PI->getPublic(*FE); !PHeader.empty()) { PHeader = PHeader.trim("<>\""); // Since most private -> public mappings happen in a verbatim way, we // check textually here. This might go wrong in presence of symlinks or // header mappings. But that's not different than rest of the places. - if(AST.tuPath().endswith(PHeader)) + if (AST.tuPath().endswith(PHeader)) return false; } } @@ -360,6 +361,7 @@ IncludeCleanerFindings computeIncludeCleanerFindings(ParsedAST &AST) { include_cleaner::Includes ConvertedIncludes = convertIncludes(SM, Includes.MainFileIncludes); const FileEntry *MainFile = SM.getFileEntryForID(SM.getMainFileID()); + auto *PreamblePatch = PreamblePatch::getPatchEntry(AST.tuPath(), SM); std::vector<include_cleaner::SymbolReference> Macros = collectMacroReferences(AST); @@ -374,7 +376,7 @@ IncludeCleanerFindings computeIncludeCleanerFindings(ParsedAST &AST) { bool Satisfied = false; for (const auto &H : Providers) { if (H.kind() == include_cleaner::Header::Physical && - H.physical() == MainFile) { + (H.physical() == MainFile || H.physical() == PreamblePatch)) { Satisfied = true; continue; } diff --git a/clang-tools-extra/clangd/Preamble.cpp b/clang-tools-extra/clangd/Preamble.cpp index 08662697a4a5..ea060bed0392 100644 --- a/clang-tools-extra/clangd/Preamble.cpp +++ b/clang-tools-extra/clangd/Preamble.cpp @@ -738,6 +738,14 @@ static std::vector<Diag> patchDiags(llvm::ArrayRef<Diag> BaselineDiags, return PatchedDiags; } +static std::string getPatchName(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), + PreamblePatch::HeaderName); + return PatchName.str().str(); +} + PreamblePatch PreamblePatch::create(llvm::StringRef FileName, const ParseInputs &Modified, const PreambleData &Baseline, @@ -776,11 +784,7 @@ PreamblePatch PreamblePatch::create(llvm::StringRef FileName, PreamblePatch PP; PP.Baseline = &Baseline; - // This shouldn't coincide with any real file name. - llvm::SmallString<128> PatchName; - llvm::sys::path::append(PatchName, llvm::sys::path::parent_path(FileName), - PreamblePatch::HeaderName); - PP.PatchFileName = PatchName.str().str(); + PP.PatchFileName = getPatchName(FileName); PP.ModifiedBounds = ModifiedScan->Bounds; llvm::raw_string_ostream Patch(PP.PatchContents); @@ -921,5 +925,13 @@ const MainFileMacros &PreamblePatch::mainFileMacros() const { return Baseline->Macros; return PatchedMacros; } + +const FileEntry *PreamblePatch::getPatchEntry(llvm::StringRef MainFilePath, + const SourceManager &SM) { + auto PatchFilePath = getPatchName(MainFilePath); + if (auto File = SM.getFileManager().getFile(PatchFilePath)) + return *File; + return nullptr; +} } // namespace clangd } // namespace clang diff --git a/clang-tools-extra/clangd/Preamble.h b/clang-tools-extra/clangd/Preamble.h index a16f497737aa..e477c01f5df5 100644 --- a/clang-tools-extra/clangd/Preamble.h +++ b/clang-tools-extra/clangd/Preamble.h @@ -30,6 +30,7 @@ #include "clang-include-cleaner/Record.h" #include "index/CanonicalIncludes.h" #include "support/Path.h" +#include "clang/Basic/SourceManager.h" #include "clang/Frontend/CompilerInvocation.h" #include "clang/Frontend/PrecompiledPreamble.h" #include "clang/Lex/Lexer.h" @@ -37,8 +38,11 @@ #include "llvm/ADT/ArrayRef.h" #include "llvm/ADT/StringRef.h" +#include <cstddef> +#include <functional> #include <memory> #include <string> +#include <utility> #include <vector> namespace clang { @@ -135,6 +139,10 @@ public: static PreamblePatch createMacroPatch(llvm::StringRef FileName, const ParseInputs &Modified, const PreambleData &Baseline); + /// Returns the FileEntry for the preamble patch of MainFilePath in SM, if + /// any. + static const FileEntry *getPatchEntry(llvm::StringRef MainFilePath, + const SourceManager &SM); /// Adjusts CI (which compiles the modified inputs) to be used with the /// baseline preamble. This is done by inserting an artifical include to the diff --git a/clang-tools-extra/clangd/unittests/PreambleTests.cpp b/clang-tools-extra/clangd/unittests/PreambleTests.cpp index 903d9ef123ee..23ba0fc3e52f 100644 --- a/clang-tools-extra/clangd/unittests/PreambleTests.cpp +++ b/clang-tools-extra/clangd/unittests/PreambleTests.cpp @@ -666,6 +666,7 @@ TEST(PreamblePatch, DiagnosticsToPreamble) { Config Cfg; Cfg.Diagnostics.AllowStalePreamble = true; Cfg.Diagnostics.UnusedIncludes = Config::IncludesPolicy::Strict; + Cfg.Diagnostics.MissingIncludes = Config::IncludesPolicy::Strict; WithContextValue WithCfg(Config::Key, std::move(Cfg)); llvm::StringMap<std::string> AdditionalFiles; @@ -699,6 +700,8 @@ $foo[[#include "foo.h"]])"); { Annotations Code("#define [[FOO]] 1\n"); // Check ranges for notes. + // This also makes sure we don't generate missing-include diagnostics + // because macros are redefined in preamble-patch. Annotations NewCode(R"(#define BARXYZ 1 #define $foo1[[FOO]] 1 void foo(); @@ -866,6 +869,27 @@ TEST(PreamblePatch, MacroAndMarkHandling) { } } +TEST(PreamblePatch, PatchFileEntry) { + Annotations Code(R"cpp(#define FOO)cpp"); + Annotations NewCode(R"cpp( +#define BAR +#define FOO)cpp"); + { + auto AST = createPatchedAST(Code.code(), Code.code()); + EXPECT_EQ( + PreamblePatch::getPatchEntry(AST->tuPath(), AST->getSourceManager()), + nullptr); + } + { + auto AST = createPatchedAST(Code.code(), NewCode.code()); + auto *FE = + PreamblePatch::getPatchEntry(AST->tuPath(), AST->getSourceManager()); + ASSERT_NE(FE, nullptr); + EXPECT_THAT(FE->getName().str(), + testing::EndsWith(PreamblePatch::HeaderName.str())); + } +} + } // namespace } // namespace clangd } // namespace clang |