[clang-tools-extra] 69c04ef - [clangd] Propagate header-guarded flag from preamble to main AST
Sam McCall via cfe-commits
cfe-commits at lists.llvm.org
Tue Jul 20 05:21:53 PDT 2021
Author: Sam McCall
Date: 2021-07-20T14:21:39+02:00
New Revision: 69c04ef95a3529614b694d7e2fd57ee817076e86
URL: https://github.com/llvm/llvm-project/commit/69c04ef95a3529614b694d7e2fd57ee817076e86
DIFF: https://github.com/llvm/llvm-project/commit/69c04ef95a3529614b694d7e2fd57ee817076e86.diff
LOG: [clangd] Propagate header-guarded flag from preamble to main AST
Fixes https://github.com/clangd/clangd/issues/377
Differential Revision: https://reviews.llvm.org/D106203
Added:
Modified:
clang-tools-extra/clangd/ParsedAST.cpp
clang-tools-extra/clangd/Preamble.cpp
clang-tools-extra/clangd/Preamble.h
clang-tools-extra/clangd/unittests/ParsedASTTests.cpp
Removed:
################################################################################
diff --git a/clang-tools-extra/clangd/ParsedAST.cpp b/clang-tools-extra/clangd/ParsedAST.cpp
index 2520062eed9b5..a60b9b6fe486a 100644
--- a/clang-tools-extra/clangd/ParsedAST.cpp
+++ b/clang-tools-extra/clangd/ParsedAST.cpp
@@ -309,6 +309,16 @@ ParsedAST::build(llvm::StringRef Filename, const ParseInputs &Inputs,
MainInput.getFile());
return None;
}
+ // If we saw an include guard in the preamble section of the main file,
+ // mark the main-file as include-guarded.
+ // This information is part of the HeaderFileInfo but is not loaded from the
+ // preamble as the file's size is part of its identity and may have changed.
+ // (The rest of HeaderFileInfo is not relevant for our purposes).
+ if (Preamble && Preamble->MainIsIncludeGuarded) {
+ const SourceManager &SM = Clang->getSourceManager();
+ const FileEntry *MainFE = SM.getFileEntryForID(SM.getMainFileID());
+ Clang->getPreprocessor().getHeaderSearchInfo().MarkFileIncludeOnce(MainFE);
+ }
// Set up ClangTidy. Must happen after BeginSourceFile() so ASTContext exists.
// Clang-tidy has some limitations to ensure reasonable performance:
diff --git a/clang-tools-extra/clangd/Preamble.cpp b/clang-tools-extra/clangd/Preamble.cpp
index 9173dcda513a6..bc8c51bcc8588 100644
--- a/clang-tools-extra/clangd/Preamble.cpp
+++ b/clang-tools-extra/clangd/Preamble.cpp
@@ -75,11 +75,20 @@ class CppFilePreambleCallbacks : public PreambleCallbacks {
CanonicalIncludes takeCanonicalIncludes() { return std::move(CanonIncludes); }
+ bool isMainFileIncludeGuarded() const { return IsMainFileIncludeGuarded; }
+
void AfterExecute(CompilerInstance &CI) override {
- if (!ParsedCallback)
- return;
- trace::Span Tracer("Running PreambleCallback");
- ParsedCallback(CI.getASTContext(), CI.getPreprocessorPtr(), CanonIncludes);
+ if (ParsedCallback) {
+ trace::Span Tracer("Running PreambleCallback");
+ ParsedCallback(CI.getASTContext(), CI.getPreprocessorPtr(),
+ CanonIncludes);
+ }
+
+ const SourceManager &SM = CI.getSourceManager();
+ const FileEntry *MainFE = SM.getFileEntryForID(SM.getMainFileID());
+ IsMainFileIncludeGuarded =
+ CI.getPreprocessor().getHeaderSearchInfo().isFileMultipleIncludeGuarded(
+ MainFE);
}
void BeforeExecute(CompilerInstance &CI) override {
@@ -121,6 +130,7 @@ class CppFilePreambleCallbacks : public PreambleCallbacks {
IncludeStructure Includes;
CanonicalIncludes CanonIncludes;
MainFileMacros Macros;
+ bool IsMainFileIncludeGuarded = false;
std::unique_ptr<CommentHandler> IWYUHandler = nullptr;
const clang::LangOptions *LangOpts = nullptr;
const SourceManager *SourceMgr = nullptr;
@@ -365,7 +375,7 @@ buildPreamble(PathRef FileName, CompilerInvocation CI,
// to read back. We rely on dynamic index for the comments instead.
CI.getPreprocessorOpts().WriteCommentListToPCH = false;
- CppFilePreambleCallbacks SerializedDeclsCollector(FileName, PreambleCallback);
+ CppFilePreambleCallbacks CapturedInfo(FileName, PreambleCallback);
auto VFS = Inputs.TFS->view(Inputs.CompileCommand.Directory);
llvm::SmallString<32> AbsFileName(FileName);
VFS->makeAbsolute(AbsFileName);
@@ -373,8 +383,7 @@ buildPreamble(PathRef FileName, CompilerInvocation CI,
auto BuiltPreamble = PrecompiledPreamble::Build(
CI, ContentsBuffer.get(), Bounds, *PreambleDiagsEngine,
StatCache->getProducingFS(VFS),
- std::make_shared<PCHContainerOperations>(), StoreInMemory,
- SerializedDeclsCollector);
+ std::make_shared<PCHContainerOperations>(), StoreInMemory, CapturedInfo);
// When building the AST for the main file, we do want the function
// bodies.
@@ -384,16 +393,17 @@ buildPreamble(PathRef FileName, CompilerInvocation CI,
vlog("Built preamble of size {0} for file {1} version {2}",
BuiltPreamble->getSize(), FileName, Inputs.Version);
std::vector<Diag> Diags = PreambleDiagnostics.take();
- return std::make_shared<PreambleData>(
+ auto Result = std::make_shared<PreambleData>(
Inputs, std::move(*BuiltPreamble), std::move(Diags),
- SerializedDeclsCollector.takeIncludes(),
- SerializedDeclsCollector.takeMacros(), std::move(StatCache),
- SerializedDeclsCollector.takeCanonicalIncludes());
- } else {
- elog("Could not build a preamble for file {0} version {1}: {2}", FileName,
- Inputs.Version, BuiltPreamble.getError().message());
- return nullptr;
+ CapturedInfo.takeIncludes(), CapturedInfo.takeMacros(),
+ std::move(StatCache), CapturedInfo.takeCanonicalIncludes());
+ Result->MainIsIncludeGuarded = CapturedInfo.isMainFileIncludeGuarded();
+ return Result;
}
+
+ elog("Could not build a preamble for file {0} version {1}: {2}", FileName,
+ Inputs.Version, BuiltPreamble.getError().message());
+ return nullptr;
}
bool isPreambleCompatible(const PreambleData &Preamble,
diff --git a/clang-tools-extra/clangd/Preamble.h b/clang-tools-extra/clangd/Preamble.h
index 5b9d17840214e..fa48d6753528c 100644
--- a/clang-tools-extra/clangd/Preamble.h
+++ b/clang-tools-extra/clangd/Preamble.h
@@ -69,6 +69,9 @@ struct PreambleData {
// When reusing a preamble, this cache can be consumed to save IO.
std::unique_ptr<PreambleFileStatusCache> StatCache;
CanonicalIncludes CanonIncludes;
+ // Whether there was a (possibly-incomplete) include-guard on the main file.
+ // We need to propagate this information "by hand" to subsequent parses.
+ bool MainIsIncludeGuarded = false;
};
using PreambleParsedCallback =
diff --git a/clang-tools-extra/clangd/unittests/ParsedASTTests.cpp b/clang-tools-extra/clangd/unittests/ParsedASTTests.cpp
index 1c960c8944cbf..365dbc581c201 100644
--- a/clang-tools-extra/clangd/unittests/ParsedASTTests.cpp
+++ b/clang-tools-extra/clangd/unittests/ParsedASTTests.cpp
@@ -670,7 +670,7 @@ TEST(ParsedASTTest, HeaderGuards) {
EXPECT_FALSE(mainIsGuarded(TU.build())); // FIXME: true
TU.Code = once(";");
- EXPECT_FALSE(mainIsGuarded(TU.build())); // FIXME: true
+ EXPECT_TRUE(mainIsGuarded(TU.build()));
TU.Code = R"cpp(
;
@@ -727,7 +727,7 @@ TEST(ParsedASTTest, HeaderGuardsSelfInclude) {
)cpp";
AST = TU.build();
EXPECT_THAT(*AST.getDiagnostics(), IsEmpty());
- EXPECT_FALSE(mainIsGuarded(AST)); // FIXME: true
+ EXPECT_TRUE(mainIsGuarded(AST));
TU.Code = R"cpp(
#pragma once
@@ -757,7 +757,7 @@ TEST(ParsedASTTest, HeaderGuardsSelfInclude) {
AST = TU.build();
EXPECT_THAT(*AST.getDiagnostics(),
ElementsAre(Diag("recursively when building a preamble")));
- EXPECT_FALSE(mainIsGuarded(AST)); // FIXME: true
+ EXPECT_TRUE(mainIsGuarded(AST));
TU.Code = R"cpp(
#ifndef GUARD
@@ -814,7 +814,7 @@ TEST(ParsedASTTest, HeaderGuardsSelfInclude) {
AST = TU.build();
EXPECT_THAT(*AST.getDiagnostics(),
ElementsAre(Diag("recursively when building a preamble")));
- EXPECT_FALSE(mainIsGuarded(AST)); // FIXME: true
+ EXPECT_TRUE(mainIsGuarded(AST));
TU.Code = R"cpp(
#include "self.h" // error-ok
@@ -863,9 +863,7 @@ TEST(ParsedASTTest, HeaderGuardsImplIface) {
// need to transfer it to the main file's HeaderFileInfo.
TU.Code = once(Interface);
AST = TU.build();
- // FIXME: empty
- EXPECT_THAT(*AST.getDiagnostics(),
- ElementsAre(Diag("in included file: redefinition of 'Traits'")));
+ EXPECT_THAT(*AST.getDiagnostics(), IsEmpty());
EXPECT_TRUE(mainIsGuarded(AST));
// Editing the implementation file, which is not include guarded.
More information about the cfe-commits
mailing list