Skip to content

Commit 778a5e9

Browse files
committed
[include-cleaner] Introduce support for always_keep pragma
Differential Revision: https://reviews.llvm.org/D156122
1 parent acdc503 commit 778a5e9

File tree

6 files changed

+78
-38
lines changed

6 files changed

+78
-38
lines changed

clang-tools-extra/clang-tidy/misc/IncludeCleanerCheck.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -143,7 +143,7 @@ void IncludeCleanerCheck::check(const MatchFinder::MatchResult &Result) {
143143
RecordedPreprocessor.Includes.all()) {
144144
if (Used.contains(&I) || !I.Resolved)
145145
continue;
146-
if (RecordedPI.shouldKeep(I.Line))
146+
if (RecordedPI.shouldKeep(I.Line) || RecordedPI.shouldKeep(*I.Resolved))
147147
continue;
148148
// Check if main file is the public interface for a private header. If so
149149
// we shouldn't diagnose it as unused.

clang-tools-extra/clangd/IncludeCleaner.cpp

Lines changed: 9 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -68,21 +68,20 @@ bool isIgnored(llvm::StringRef HeaderPath, HeaderFilter IgnoreHeaders) {
6868
return false;
6969
}
7070

71-
bool mayConsiderUnused(
72-
const Inclusion &Inc, ParsedAST &AST,
73-
const include_cleaner::PragmaIncludes *PI) {
74-
if (PI && PI->shouldKeep(Inc.HashLine + 1))
75-
return false;
76-
// FIXME(kirillbobyrev): We currently do not support the umbrella headers.
77-
// System headers are likely to be standard library headers.
78-
// Until we have good support for umbrella headers, don't warn about them.
79-
if (Inc.Written.front() == '<')
80-
return tooling::stdlib::Header::named(Inc.Written).has_value();
71+
bool mayConsiderUnused(const Inclusion &Inc, ParsedAST &AST,
72+
const include_cleaner::PragmaIncludes *PI) {
8173
assert(Inc.HeaderID);
8274
auto HID = static_cast<IncludeStructure::HeaderID>(*Inc.HeaderID);
8375
auto FE = AST.getSourceManager().getFileManager().getFileRef(
8476
AST.getIncludeStructure().getRealPath(HID));
8577
assert(FE);
78+
if (PI && (PI->shouldKeep(Inc.HashLine + 1) || PI->shouldKeep(*FE)))
79+
return false;
80+
// FIXME(kirillbobyrev): We currently do not support the umbrella headers.
81+
// System headers are likely to be standard library headers.
82+
// Until we have good support for umbrella headers, don't warn about them.
83+
if (Inc.Written.front() == '<')
84+
return tooling::stdlib::Header::named(Inc.Written).has_value();
8685
if (PI) {
8786
// Check if main file is the public interface for a private header. If so we
8887
// shouldn't diagnose it as unused.

clang-tools-extra/include-cleaner/include/clang-include-cleaner/Record.h

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -59,9 +59,8 @@ class PragmaIncludes {
5959

6060
/// Returns true if the given #include of the main-file should never be
6161
/// removed.
62-
bool shouldKeep(unsigned HashLineNumber) const {
63-
return ShouldKeep.contains(HashLineNumber);
64-
}
62+
bool shouldKeep(unsigned HashLineNumber) const;
63+
bool shouldKeep(const FileEntry *FE) const;
6564

6665
/// Returns the public mapping include for the given physical header file.
6766
/// Returns "" if there is none.
@@ -113,6 +112,8 @@ class PragmaIncludes {
113112

114113
/// Contains all non self-contained files detected during the parsing.
115114
llvm::DenseSet<llvm::sys::fs::UniqueID> NonSelfContainedFiles;
115+
// Files with an always_keep pragma.
116+
llvm::DenseSet<llvm::sys::fs::UniqueID> AlwaysKeep;
116117

117118
/// Owns the strings.
118119
llvm::BumpPtrAllocator Arena;

clang-tools-extra/include-cleaner/lib/Analysis.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -91,7 +91,7 @@ analyze(llvm::ArrayRef<Decl *> ASTRoots,
9191
HeaderFilter(I.Resolved->getFileEntry().tryGetRealPathName()))
9292
continue;
9393
if (PI) {
94-
if (PI->shouldKeep(I.Line))
94+
if (PI->shouldKeep(I.Line) || PI->shouldKeep(*I.Resolved))
9595
continue;
9696
// Check if main file is the public interface for a private header. If so
9797
// we shouldn't diagnose it as unused.

clang-tools-extra/include-cleaner/lib/Record.cpp

Lines changed: 48 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,9 @@
1111
#include "clang/AST/ASTConsumer.h"
1212
#include "clang/AST/ASTContext.h"
1313
#include "clang/AST/DeclGroup.h"
14+
#include "clang/Basic/FileEntry.h"
15+
#include "clang/Basic/LLVM.h"
16+
#include "clang/Basic/SourceLocation.h"
1417
#include "clang/Basic/SourceManager.h"
1518
#include "clang/Basic/Specifiers.h"
1619
#include "clang/Frontend/CompilerInstance.h"
@@ -20,8 +23,17 @@
2023
#include "clang/Lex/Preprocessor.h"
2124
#include "clang/Tooling/Inclusions/HeaderAnalysis.h"
2225
#include "clang/Tooling/Inclusions/StandardLibrary.h"
26+
#include "llvm/ADT/ArrayRef.h"
27+
#include "llvm/ADT/STLExtras.h"
28+
#include "llvm/ADT/SmallVector.h"
2329
#include "llvm/ADT/StringRef.h"
30+
#include "llvm/Support/Allocator.h"
31+
#include "llvm/Support/Error.h"
32+
#include "llvm/Support/StringSaver.h"
33+
#include <algorithm>
34+
#include <assert.h>
2435
#include <memory>
36+
#include <optional>
2537
#include <utility>
2638
#include <vector>
2739

@@ -262,32 +274,14 @@ class PragmaIncludes::RecordPragma : public PPCallbacks, public CommentHandler {
262274
if (!Pragma)
263275
return false;
264276

265-
if (Pragma->consume_front("private")) {
266-
auto *FE = SM.getFileEntryForID(SM.getFileID(Range.getBegin()));
267-
if (!FE)
268-
return false;
269-
StringRef PublicHeader;
270-
if (Pragma->consume_front(", include ")) {
271-
// We always insert using the spelling from the pragma.
272-
PublicHeader = save(Pragma->startswith("<") || Pragma->startswith("\"")
273-
? (*Pragma)
274-
: ("\"" + *Pragma + "\"").str());
275-
}
276-
Out->IWYUPublic.insert({FE->getLastRef().getUniqueID(), PublicHeader});
277-
return false;
278-
}
279-
FileID CommentFID = SM.getFileID(Range.getBegin());
280-
int CommentLine = SM.getLineNumber(SM.getFileID(Range.getBegin()),
281-
SM.getFileOffset(Range.getBegin()));
277+
auto [CommentFID, CommentOffset] = SM.getDecomposedLoc(Range.getBegin());
278+
int CommentLine = SM.getLineNumber(CommentFID, CommentOffset);
279+
auto Filename = SM.getBufferName(Range.getBegin());
282280
// Record export pragma.
283281
if (Pragma->startswith("export")) {
284-
ExportStack.push_back({CommentLine, CommentFID,
285-
save(SM.getFileEntryForID(CommentFID)->getName()),
286-
false});
282+
ExportStack.push_back({CommentLine, CommentFID, save(Filename), false});
287283
} else if (Pragma->startswith("begin_exports")) {
288-
ExportStack.push_back({CommentLine, CommentFID,
289-
save(SM.getFileEntryForID(CommentFID)->getName()),
290-
true});
284+
ExportStack.push_back({CommentLine, CommentFID, save(Filename), true});
291285
} else if (Pragma->startswith("end_exports")) {
292286
// FIXME: be robust on unmatching cases. We should only pop the stack if
293287
// the begin_exports and end_exports is in the same file.
@@ -307,6 +301,29 @@ class PragmaIncludes::RecordPragma : public PPCallbacks, public CommentHandler {
307301
KeepStack.pop_back();
308302
}
309303
}
304+
305+
auto FE = SM.getFileEntryRefForID(CommentFID);
306+
if (!FE) {
307+
// FIXME: Support IWYU pragmas in virtual files. Our mappings rely on
308+
// "persistent" UniqueIDs and that is not the case for virtual files.
309+
return false;
310+
}
311+
auto CommentUID = FE->getUniqueID();
312+
if (Pragma->consume_front("private")) {
313+
StringRef PublicHeader;
314+
if (Pragma->consume_front(", include ")) {
315+
// We always insert using the spelling from the pragma.
316+
PublicHeader = save(Pragma->startswith("<") || Pragma->startswith("\"")
317+
? (*Pragma)
318+
: ("\"" + *Pragma + "\"").str());
319+
}
320+
Out->IWYUPublic.insert({CommentUID, PublicHeader});
321+
return false;
322+
}
323+
if (Pragma->consume_front("always_keep")) {
324+
Out->AlwaysKeep.insert(CommentUID);
325+
return false;
326+
}
310327
return false;
311328
}
312329

@@ -401,6 +418,14 @@ bool PragmaIncludes::isPrivate(const FileEntry *FE) const {
401418
return IWYUPublic.contains(FE->getUniqueID());
402419
}
403420

421+
bool PragmaIncludes::shouldKeep(unsigned HashLineNumber) const {
422+
return ShouldKeep.contains(HashLineNumber);
423+
}
424+
425+
bool PragmaIncludes::shouldKeep(const FileEntry *FE) const {
426+
return AlwaysKeep.contains(FE->getUniqueID());
427+
}
428+
404429
namespace {
405430
template <typename T> bool isImplicitTemplateSpecialization(const Decl *D) {
406431
if (const auto *TD = dyn_cast<T>(D))

clang-tools-extra/include-cleaner/unittests/RecordTest.cpp

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -502,5 +502,20 @@ TEST_F(PragmaIncludeTest, SelfContained) {
502502
EXPECT_FALSE(PI.isSelfContained(FM.getFile("unguarded.h").get()));
503503
}
504504

505+
TEST_F(PragmaIncludeTest, AlwaysKeep) {
506+
Inputs.Code = R"cpp(
507+
#include "always_keep.h"
508+
#include "usual.h"
509+
)cpp";
510+
Inputs.ExtraFiles["always_keep.h"] = R"cpp(
511+
#pragma once
512+
// IWYU pragma: always_keep
513+
)cpp";
514+
Inputs.ExtraFiles["usual.h"] = "#pragma once";
515+
TestAST Processed = build();
516+
auto &FM = Processed.fileManager();
517+
EXPECT_TRUE(PI.shouldKeep(FM.getFile("always_keep.h").get()));
518+
EXPECT_FALSE(PI.shouldKeep(FM.getFile("usual.h").get()));
519+
}
505520
} // namespace
506521
} // namespace clang::include_cleaner

0 commit comments

Comments
 (0)