[clang-tools-extra] 144562e - [clangd] Treat preamble patch as main file for include-cleaner analysis
Kadir Cetinkaya via cfe-commits
cfe-commits at lists.llvm.org
Wed Apr 12 12:06:14 PDT 2023
Author: Kadir Cetinkaya
Date: 2023-04-12T21:03:21+02:00
New Revision: 144562e678d932dc2685f8a83aeb2229bc96d71a
URL: https://github.com/llvm/llvm-project/commit/144562e678d932dc2685f8a83aeb2229bc96d71a
DIFF: https://github.com/llvm/llvm-project/commit/144562e678d932dc2685f8a83aeb2229bc96d71a.diff
LOG: [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
Added:
Modified:
clang-tools-extra/clangd/IncludeCleaner.cpp
clang-tools-extra/clangd/Preamble.cpp
clang-tools-extra/clangd/Preamble.h
clang-tools-extra/clangd/unittests/PreambleTests.cpp
Removed:
################################################################################
diff --git a/clang-tools-extra/clangd/IncludeCleaner.cpp b/clang-tools-extra/clangd/IncludeCleaner.cpp
index ee645efa6e6e1..e645b1cce6a09 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
diff erent 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 08662697a4a5c..ea060bed03922 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 a16f497737aa3..e477c01f5df50 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 @@ class PreamblePatch {
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 903d9ef123eea..23ba0fc3e52fc 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 @@ TEST(PreamblePatch, DiagnosticsToPreamble) {
{
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
More information about the cfe-commits
mailing list