cjdb created this revision.
cjdb added reviewers: aaron.ballman, rsmith, gbiv, dblaikie, cor3ntin.
Herald added a reviewer: george.burgess.iv.
Herald added subscribers: usaxena95, kadircet.
cjdb requested review of this revision.
Herald added subscribers: cfe-commits, ilya-biryukov.
Herald added a project: clang.
`#pragma clang include_instead(<header>)` is a pragma that can be used
by system headers (and only system headers) to indicate to a tool that
the file containing said pragma is an implementation-detail header and
should not be directly included by user code.
The library alternative is very messy code that can be seen in the first
diff of D106124 <https://reviews.llvm.org/D106124>, and we'd rather avoid that
with something more
universal.
This patch takes the first step by warning a user when they include a
detail header in their code, and suggests alternative headers that the
user should include instead. Future work will involve adding a fixit to
automate the process, as well as cleaning up modules diagnostics to not
suggest said detail headers. Other tools, such as clangd can also take
advantage of this pragma to add the correct user headers.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D106394
Files:
clang/include/clang/Basic/DiagnosticGroups.td
clang/include/clang/Basic/DiagnosticLexKinds.td
clang/include/clang/Lex/HeaderSearch.h
clang/include/clang/Lex/Preprocessor.h
clang/include/clang/Lex/PreprocessorLexer.h
clang/lib/Lex/PPDirectives.cpp
clang/lib/Lex/PPLexerChange.cpp
clang/lib/Lex/Pragma.cpp
clang/test/Preprocessor/Inputs/include_instead/include_instead-bad-syntax.h
clang/test/Preprocessor/Inputs/include_instead/include_instead-non-system-header.h
clang/test/Preprocessor/Inputs/include_instead/include_instead-nowarn.h
clang/test/Preprocessor/Inputs/include_instead/include_instead-warns-three-alternatives.h
clang/test/Preprocessor/Inputs/include_instead/include_instead-warns-two-alternatives.h
clang/test/Preprocessor/Inputs/include_instead/include_instead-warns.h
clang/test/Preprocessor/Inputs/include_instead/malloc.h
clang/test/Preprocessor/include_instead.cpp
Index: clang/test/Preprocessor/include_instead.cpp
===================================================================
--- /dev/null
+++ clang/test/Preprocessor/include_instead.cpp
@@ -0,0 +1,15 @@
+// RUN: %clang_cc1 -fsyntax-only -verify -I %S/Inputs/include_instead %s
+
+#include <include_instead-bad-syntax.h>
+#include <include_instead-non-system-header.h>
+
+#include <include_instead-warns.h>
+// expected-warning@-1{{header '<include_instead-warns.h>' is an implementation detail; #include '<malloc.h>' instead}}
+
+#include "include_instead-warns-two-alternatives.h"
+// expected-warning@-1{{header '"include_instead-warns-two-alternatives.h"' is an implementation detail; #include either '"include_instead-nowarn.h"' or '<malloc.h>' instead}}
+
+#include <include_instead-warns-three-alternatives.h>
+// expected-warning@-1{{header '<include_instead-warns-three-alternatives.h>' is an implementation detail; #include one of {'"include_instead-nowarn.h"', '"include_instead-warns-two-alternatives.h"', '<malloc.h>'} instead}}
+
+#include <include_instead-nowarn.h>
Index: clang/test/Preprocessor/Inputs/include_instead/malloc.h
===================================================================
--- /dev/null
+++ clang/test/Preprocessor/Inputs/include_instead/malloc.h
@@ -0,0 +1 @@
+// This file simply needs to exist.
Index: clang/test/Preprocessor/Inputs/include_instead/include_instead-warns.h
===================================================================
--- /dev/null
+++ clang/test/Preprocessor/Inputs/include_instead/include_instead-warns.h
@@ -0,0 +1,3 @@
+#pragma GCC system_header
+
+#pragma clang include_instead(<malloc.h>)
Index: clang/test/Preprocessor/Inputs/include_instead/include_instead-warns-two-alternatives.h
===================================================================
--- /dev/null
+++ clang/test/Preprocessor/Inputs/include_instead/include_instead-warns-two-alternatives.h
@@ -0,0 +1,4 @@
+#pragma GCC system_header
+
+#pragma clang include_instead(<malloc.h>)
+#pragma clang include_instead("include_instead-nowarn.h")
Index: clang/test/Preprocessor/Inputs/include_instead/include_instead-warns-three-alternatives.h
===================================================================
--- /dev/null
+++ clang/test/Preprocessor/Inputs/include_instead/include_instead-warns-three-alternatives.h
@@ -0,0 +1,5 @@
+#pragma GCC system_header
+
+#pragma clang include_instead(<malloc.h>)
+#pragma clang include_instead("include_instead-nowarn.h")
+#pragma clang include_instead("include_instead-warns-two-alternatives.h")
Index: clang/test/Preprocessor/Inputs/include_instead/include_instead-nowarn.h
===================================================================
--- /dev/null
+++ clang/test/Preprocessor/Inputs/include_instead/include_instead-nowarn.h
@@ -0,0 +1,3 @@
+#pragma GCC system_header
+
+#include <include_instead-warns.h> // no warning expected
Index: clang/test/Preprocessor/Inputs/include_instead/include_instead-non-system-header.h
===================================================================
--- /dev/null
+++ clang/test/Preprocessor/Inputs/include_instead/include_instead-non-system-header.h
@@ -0,0 +1,2 @@
+#pragma clang include_instead(<malloc.h>)
+// expected-warning@-1{{'#pragma include_instead' ignored outside of system headers}}
Index: clang/test/Preprocessor/Inputs/include_instead/include_instead-bad-syntax.h
===================================================================
--- /dev/null
+++ clang/test/Preprocessor/Inputs/include_instead/include_instead-bad-syntax.h
@@ -0,0 +1,10 @@
+#pragma GCC system_header
+
+#pragma clang include_instead <malloc.h>
+// expected-warning@-1{{'#pragma include_instead' expects '(' as its next token; got '<' instead}}
+
+#pragma clang include_instead(<malloc.h>]
+// expected-warning@-1{{'#pragma include_instead' expects ')' as its next token; got ']' instead}}
+
+#pragma clang include_instead(<include_instead_does_not_exist.h>)
+// expected-warning@-1{{'include_instead_does_not_exist.h' file not found}}
Index: clang/lib/Lex/Pragma.cpp
===================================================================
--- clang/lib/Lex/Pragma.cpp
+++ clang/lib/Lex/Pragma.cpp
@@ -13,6 +13,7 @@
#include "clang/Lex/Pragma.h"
#include "clang/Basic/Diagnostic.h"
+#include "clang/Basic/DiagnosticLex.h"
#include "clang/Basic/FileManager.h"
#include "clang/Basic/IdentifierTable.h"
#include "clang/Basic/LLVM.h"
@@ -35,11 +36,12 @@
#include "clang/Lex/TokenLexer.h"
#include "llvm/ADT/ArrayRef.h"
#include "llvm/ADT/DenseMap.h"
+#include "llvm/ADT/Optional.h"
#include "llvm/ADT/STLExtras.h"
#include "llvm/ADT/SmallString.h"
#include "llvm/ADT/SmallVector.h"
-#include "llvm/ADT/StringSwitch.h"
#include "llvm/ADT/StringRef.h"
+#include "llvm/ADT/StringSwitch.h"
#include "llvm/Support/Compiler.h"
#include "llvm/Support/ErrorHandling.h"
#include "llvm/Support/Timer.h"
@@ -495,40 +497,94 @@
SrcMgr::C_System);
}
-/// HandlePragmaDependency - Handle \#pragma GCC dependency "foo" blah.
-void Preprocessor::HandlePragmaDependency(Token &DependencyTok) {
+static llvm::Optional<Token> LexHeader(Preprocessor &PP,
+ Optional<FileEntryRef> &File,
+ bool SuppressIncludeNotFoundError,
+ bool FatalError) {
Token FilenameTok;
- if (LexHeaderName(FilenameTok, /*AllowConcatenation*/false))
- return;
+ if (PP.LexHeaderName(FilenameTok, /*AllowConcatenation*/ false))
+ return {};
// If the next token wasn't a header-name, diagnose the error.
if (FilenameTok.isNot(tok::header_name)) {
- Diag(FilenameTok.getLocation(), diag::err_pp_expects_filename);
- return;
+ PP.Diag(FilenameTok.getLocation(), diag::err_pp_expects_filename);
+ return {};
}
// Reserve a buffer to get the spelling.
SmallString<128> FilenameBuffer;
bool Invalid = false;
- StringRef Filename = getSpelling(FilenameTok, FilenameBuffer, &Invalid);
+ StringRef Filename = PP.getSpelling(FilenameTok, FilenameBuffer, &Invalid);
if (Invalid)
- return;
+ return {};
bool isAngled =
- GetIncludeFilenameSpelling(FilenameTok.getLocation(), Filename);
+ PP.GetIncludeFilenameSpelling(FilenameTok.getLocation(), Filename);
// If GetIncludeFilenameSpelling set the start ptr to null, there was an
// error.
if (Filename.empty())
- return;
+ return {};
// Search include directories for this file.
const DirectoryLookup *CurDir;
- Optional<FileEntryRef> File =
- LookupFile(FilenameTok.getLocation(), Filename, isAngled, nullptr,
- nullptr, CurDir, nullptr, nullptr, nullptr, nullptr, nullptr);
+ File = PP.LookupFile(FilenameTok.getLocation(), Filename, isAngled, nullptr,
+ nullptr, CurDir, nullptr, nullptr, nullptr, nullptr,
+ nullptr);
if (!File) {
if (!SuppressIncludeNotFoundError)
- Diag(FilenameTok, diag::err_pp_file_not_found) << Filename;
+ PP.Diag(FilenameTok, FatalError
+ ? diag::err_pp_file_not_found
+ : diag::pp_pragma_include_instead_file_not_found)
+ << Filename;
+ return {};
+ }
+
+ return FilenameTok;
+}
+
+/// HandlePragmaIncludeInstead - Handle \#pragma clang include_instead(header).
+void Preprocessor::HandlePragmaIncludeInstead(Token &Tok) {
+ // Get the current file lexer we're looking at. Ignore _Pragma 'files' etc.
+ PreprocessorLexer *TheLexer = getCurrentFileLexer();
+
+ if (!SourceMgr.isInSystemHeader(Tok.getLocation())) {
+ Diag(Tok, diag::pp_pragma_include_instead_not_sysheader);
+ return;
+ }
+
+ Lex(Tok);
+ if (Tok.isNot(tok::l_paren)) {
+ const std::string spelling = getSpelling(Tok);
+ Diag(Tok, diag::pp_pragma_include_instead_unexpected_token)
+ << "(" << (spelling == "\n" ? "\\n" : spelling);
+ return;
+ }
+
+ Optional<FileEntryRef> File;
+ llvm::Optional<Token> FilenameTok = LexHeader(
+ *this, File, SuppressIncludeNotFoundError, /*FatalError=*/false);
+ if (!FilenameTok) {
+ return;
+ }
+
+ Lex(Tok);
+ if (Tok.isNot(tok::r_paren)) {
+ const std::string spelling = getSpelling(Tok);
+ Diag(Tok, diag::pp_pragma_include_instead_unexpected_token)
+ << ")" << spelling;
+ return;
+ }
+
+ HeaderInfo.AddFileAlias(TheLexer->getFileEntry(),
+ FilenameTok->getRawIdentifier());
+}
+
+/// HandlePragmaDependency - Handle \#pragma GCC dependency "foo" blah.
+void Preprocessor::HandlePragmaDependency(Token &DependencyTok) {
+ Optional<FileEntryRef> File;
+ llvm::Optional<Token> FilenameTok =
+ LexHeader(*this, File, SuppressIncludeNotFoundError, /*FatalError=*/true);
+ if (!FilenameTok) {
return;
}
@@ -547,7 +603,7 @@
// Remove the trailing ' ' if present.
if (!Message.empty())
Message.erase(Message.end()-1);
- Diag(FilenameTok, diag::pp_out_of_date_dependency) << Message;
+ Diag(*FilenameTok, diag::pp_out_of_date_dependency) << Message;
}
}
@@ -1022,6 +1078,18 @@
}
};
+/// PragmaIncludeInsteadHandler - "\#pragma include_instead(header)" marks the
+/// current file as non-includable if the including header is not a system
+/// header.
+struct PragmaIncludeInsteadHandler : public PragmaHandler {
+ PragmaIncludeInsteadHandler() : PragmaHandler("include_instead") {}
+
+ void HandlePragma(Preprocessor &PP, PragmaIntroducer Introducer,
+ Token &IIToken) override {
+ PP.HandlePragmaIncludeInstead(IIToken);
+ }
+};
+
struct PragmaDependencyHandler : public PragmaHandler {
PragmaDependencyHandler() : PragmaHandler("dependency") {}
@@ -1934,6 +2002,7 @@
// #pragma clang ...
AddPragmaHandler("clang", new PragmaPoisonHandler());
AddPragmaHandler("clang", new PragmaSystemHeaderHandler());
+ AddPragmaHandler("clang", new PragmaIncludeInsteadHandler());
AddPragmaHandler("clang", new PragmaDebugHandler());
AddPragmaHandler("clang", new PragmaDependencyHandler());
AddPragmaHandler("clang", new PragmaDiagnosticHandler("clang"));
Index: clang/lib/Lex/PPLexerChange.cpp
===================================================================
--- clang/lib/Lex/PPLexerChange.cpp
+++ clang/lib/Lex/PPLexerChange.cpp
@@ -22,6 +22,9 @@
#include "llvm/Support/FileSystem.h"
#include "llvm/Support/MemoryBufferRef.h"
#include "llvm/Support/Path.h"
+
+#include <numeric>
+
using namespace clang;
//===----------------------------------------------------------------------===//
@@ -395,6 +398,48 @@
PragmaAssumeNonNullLoc = SourceLocation();
}
+ if (CurLexer != nullptr) {
+ auto IncludeHistory = CurLexer->getIncludeHistory();
+ for (const auto &Include : IncludeHistory) {
+ const StringRef Filename = Include.getKey();
+ const auto &IncludeInfo = Include.getValue();
+ const HeaderFileInfo &Info = HeaderInfo.getFileInfo(IncludeInfo.File);
+
+ if (SourceMgr.isInSystemHeader(IncludeInfo.Location))
+ break;
+
+ if (Info.Aliases.empty())
+ continue;
+
+ switch (Info.Aliases.size()) {
+ case 1:
+ Diag(IncludeInfo.Location,
+ diag::pp_pragma_include_instead_system_reserved)
+ << Filename << 0 << Info.Aliases[0];
+ continue;
+ case 2:
+ Diag(IncludeInfo.Location,
+ diag::pp_pragma_include_instead_system_reserved)
+ << Filename << 1 << Info.Aliases[0] << Info.Aliases[1];
+ continue;
+ default: {
+ auto Quote = [](StringRef S) -> std::string {
+ return ("'" + S + "'").str();
+ };
+ std::string Aliases =
+ std::accumulate(Info.Aliases.begin() + 1, Info.Aliases.end(),
+ Quote(Info.Aliases.front()),
+ [Quote](const StringRef X, const StringRef Y) {
+ return X.str() + ", " + Quote(Y);
+ });
+ Diag(IncludeInfo.Location,
+ diag::pp_pragma_include_instead_system_reserved)
+ << Filename << 2 << ('{' + Aliases + '}');
+ }
+ }
+ }
+ }
+
bool LeavingPCHThroughHeader = false;
// If this is a #include'd file, pop it off the include stack and continue
Index: clang/lib/Lex/PPDirectives.cpp
===================================================================
--- clang/lib/Lex/PPDirectives.cpp
+++ clang/lib/Lex/PPDirectives.cpp
@@ -2022,6 +2022,11 @@
IsFrameworkFound, IsImportDecl, IsMapped, LookupFrom, LookupFromFile,
LookupFilename, RelativePath, SearchPath, SuggestedModule, isAngled);
+ // Record the header's filename for later use
+ if (File)
+ CurLexer->addInclude(FilenameTok.getRawIdentifier(), File->getFileEntry(),
+ FilenameTok.getLocation());
+
if (usingPCHWithThroughHeader() && SkippingUntilPCHThroughHeader) {
if (File && isPCHThroughHeader(&File->getFileEntry()))
SkippingUntilPCHThroughHeader = false;
Index: clang/include/clang/Lex/PreprocessorLexer.h
===================================================================
--- clang/include/clang/Lex/PreprocessorLexer.h
+++ clang/include/clang/Lex/PreprocessorLexer.h
@@ -14,11 +14,13 @@
#ifndef LLVM_CLANG_LEX_PREPROCESSORLEXER_H
#define LLVM_CLANG_LEX_PREPROCESSORLEXER_H
+#include "clang/Basic/SourceLocation.h"
+#include "clang/Lex/HeaderSearch.h"
#include "clang/Lex/MultipleIncludeOpt.h"
#include "clang/Lex/Token.h"
-#include "clang/Basic/SourceLocation.h"
#include "llvm/ADT/ArrayRef.h"
#include "llvm/ADT/SmallVector.h"
+#include "llvm/ADT/StringMap.h"
#include <cassert>
namespace clang {
@@ -74,6 +76,13 @@
/// we are currently in.
SmallVector<PPConditionalInfo, 4> ConditionalStack;
+ struct IncludeInfo {
+ const FileEntry *File;
+ SourceLocation Location;
+ };
+ // A complete history of all the files included by the current file.
+ llvm::StringMap<IncludeInfo> IncludeHistory;
+
PreprocessorLexer() : FID() {}
PreprocessorLexer(Preprocessor *pp, FileID fid);
virtual ~PreprocessorLexer() = default;
@@ -175,6 +184,15 @@
ConditionalStack.clear();
ConditionalStack.append(CL.begin(), CL.end());
}
+
+ void addInclude(const StringRef Filename, const FileEntry &File,
+ const SourceLocation Location) {
+ IncludeHistory.insert({Filename, {&File, Location}});
+ }
+
+ const llvm::StringMap<IncludeInfo> &getIncludeHistory() const {
+ return IncludeHistory;
+ }
};
} // namespace clang
Index: clang/include/clang/Lex/Preprocessor.h
===================================================================
--- clang/include/clang/Lex/Preprocessor.h
+++ clang/include/clang/Lex/Preprocessor.h
@@ -2370,6 +2370,7 @@
void HandlePragmaMark(Token &MarkTok);
void HandlePragmaPoison();
void HandlePragmaSystemHeader(Token &SysHeaderTok);
+ void HandlePragmaIncludeInstead(Token &Tok);
void HandlePragmaDependency(Token &DependencyTok);
void HandlePragmaPushMacro(Token &Tok);
void HandlePragmaPopMacro(Token &Tok);
Index: clang/include/clang/Lex/HeaderSearch.h
===================================================================
--- clang/include/clang/Lex/HeaderSearch.h
+++ clang/include/clang/Lex/HeaderSearch.h
@@ -20,9 +20,10 @@
#include "clang/Lex/ModuleMap.h"
#include "llvm/ADT/ArrayRef.h"
#include "llvm/ADT/DenseMap.h"
+#include "llvm/ADT/SmallString.h"
#include "llvm/ADT/StringMap.h"
-#include "llvm/ADT/StringSet.h"
#include "llvm/ADT/StringRef.h"
+#include "llvm/ADT/StringSet.h"
#include "llvm/Support/Allocator.h"
#include <cassert>
#include <cstddef>
@@ -110,6 +111,11 @@
/// of the framework.
StringRef Framework;
+ /// List of aliases that this header is known as.
+ /// Most headers should only have at most one alias, but a handful
+ /// have two.
+ llvm::SmallVector<llvm::SmallString<32>, 2> Aliases;
+
HeaderFileInfo()
: isImport(false), isPragmaOnce(false), DirInfo(SrcMgr::C_User),
External(false), isModuleHeader(false), isCompilingModuleHeader(false),
@@ -453,6 +459,15 @@
getFileInfo(File).DirInfo = SrcMgr::C_System;
}
+ void AddFileAlias(const FileEntry *File, StringRef Alias) {
+ llvm::SmallVector<llvm::SmallString<32>, 2> &Aliases =
+ getFileInfo(File).Aliases;
+ auto InsertionPoint = llvm::lower_bound(Aliases, Alias);
+ if (InsertionPoint != Aliases.end() && *InsertionPoint == Alias)
+ return;
+ Aliases.insert(InsertionPoint, Alias);
+ }
+
/// Mark the specified file as part of a module.
void MarkFileModuleHeader(const FileEntry *FE,
ModuleMap::ModuleHeaderRole Role,
Index: clang/include/clang/Basic/DiagnosticLexKinds.td
===================================================================
--- clang/include/clang/Basic/DiagnosticLexKinds.td
+++ clang/include/clang/Basic/DiagnosticLexKinds.td
@@ -300,6 +300,22 @@
def pp_pragma_sysheader_in_main_file : Warning<
"#pragma system_header ignored in main file">,
InGroup<DiagGroup<"pragma-system-header-outside-header">>;
+def pp_pragma_include_instead_not_sysheader : Warning<
+ "'#pragma include_instead' ignored outside of system headers">,
+ InGroup<PragmaIncludeInstead>,
+ ShowInSystemHeader;
+def pp_pragma_include_instead_unexpected_token : Warning<
+ "'#pragma include_instead' expects '%0' as its next token; got '%1' instead">,
+ InGroup<PragmaIncludeInstead>,
+ ShowInSystemHeader;
+def pp_pragma_include_instead_file_not_found : Warning<
+ "'%0' file not found">,
+ InGroup<PragmaIncludeInstead>,
+ ShowInSystemHeader;
+def pp_pragma_include_instead_system_reserved : Warning<
+ "header '%0' is an implementation detail; #include %select{'%2'|either '%2' or '%3'|one of %2}1 instead">,
+ InGroup<PragmaIncludeInstead>,
+ ShowInSystemHeader;
def pp_poisoning_existing_macro : Warning<"poisoning existing macro">;
def pp_out_of_date_dependency : Warning<
"current file is older than dependency %0">;
Index: clang/include/clang/Basic/DiagnosticGroups.td
===================================================================
--- clang/include/clang/Basic/DiagnosticGroups.td
+++ clang/include/clang/Basic/DiagnosticGroups.td
@@ -693,8 +693,10 @@
def PragmaClangAttribute : DiagGroup<"pragma-clang-attribute">;
def PragmaPackSuspiciousInclude : DiagGroup<"pragma-pack-suspicious-include">;
def PragmaPack : DiagGroup<"pragma-pack", [PragmaPackSuspiciousInclude]>;
+def PragmaIncludeInstead : DiagGroup<"pragma-include-instead">;
def Pragmas : DiagGroup<"pragmas", [UnknownPragmas, IgnoredPragmas,
- PragmaClangAttribute, PragmaPack]>;
+ PragmaClangAttribute, PragmaPack,
+ PragmaIncludeInstead]>;
def UnknownWarningOption : DiagGroup<"unknown-warning-option">;
def NSobjectAttribute : DiagGroup<"NSObject-attribute">;
def NSConsumedMismatch : DiagGroup<"nsconsumed-mismatch">;
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
