-
Notifications
You must be signed in to change notification settings - Fork 15.2k
[Clang] Add #pragma clang deprecated_header #168041
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
de98460 to
c64e4db
Compare
|
@llvm/pr-subscribers-clang Author: Nikolas Klauser (philnik777) ChangesThis pragma allows marking headers as deprecated, which significantly improves the diagnostics issued compared to the traditional Specifically, the warning is issued at the place the header is included instead inside the header. This allows suppressing the warning more locally if required. Using the pragma also moves the diagnostic into Full diff: https://github.com/llvm/llvm-project/pull/168041.diff 9 Files Affected:
diff --git a/clang/include/clang/Basic/DiagnosticLexKinds.td b/clang/include/clang/Basic/DiagnosticLexKinds.td
index 417187222e448..c587a0e861949 100644
--- a/clang/include/clang/Basic/DiagnosticLexKinds.td
+++ b/clang/include/clang/Basic/DiagnosticLexKinds.td
@@ -666,6 +666,11 @@ def warn_pragma_deprecated_macro_use :
ExtWarn<"macro %0 has been marked as deprecated%select{|: %2}1">,
InGroup<DeprecatedPragma>;
+// - #pragma depcreated_header
+def warn_pragma_deprecated_header : Warning<
+ "header is deprecated%select{|: %1}0">,
+ InGroup<Deprecated>;
+
// - #pragma clang restrict_expansion(...)
def warn_pragma_restrict_expansion_macro_use :
ExtWarn<"macro %0 has been marked as unsafe for use in headers"
diff --git a/clang/include/clang/Lex/HeaderSearch.h b/clang/include/clang/Lex/HeaderSearch.h
index 850aea41c4c3b..3a3d5498a2d44 100644
--- a/clang/include/clang/Lex/HeaderSearch.h
+++ b/clang/include/clang/Lex/HeaderSearch.h
@@ -80,6 +80,9 @@ struct HeaderFileInfo {
LLVM_PREFERRED_TYPE(SrcMgr::CharacteristicKind)
unsigned DirInfo : 3;
+ LLVM_PREFERRED_TYPE(bool)
+ unsigned IsDeprecated : 1;
+
/// Whether this header file info was supplied by an external source,
/// and has not changed since.
LLVM_PREFERRED_TYPE(bool)
@@ -124,9 +127,9 @@ struct HeaderFileInfo {
HeaderFileInfo()
: IsLocallyIncluded(false), isImport(false), isPragmaOnce(false),
- DirInfo(SrcMgr::C_User), External(false), isModuleHeader(false),
- isTextualModuleHeader(false), isCompilingModuleHeader(false),
- Resolved(false), IsValid(false) {}
+ DirInfo(SrcMgr::C_User), IsDeprecated(false), External(false),
+ isModuleHeader(false), isTextualModuleHeader(false),
+ isCompilingModuleHeader(false), Resolved(false), IsValid(false) {}
/// Retrieve the controlling macro for this header file, if
/// any.
@@ -356,6 +359,9 @@ class HeaderSearch {
// A map of discovered headers with their associated include file name.
llvm::DenseMap<const FileEntry *, llvm::SmallString<64>> IncludeNames;
+ // A map from a file to its deprecation message
+ llvm::DenseMap<const FileEntry *, std::string> DeprecationMessages;
+
/// Uniqued set of framework names, which is used to track which
/// headers were included as framework headers.
llvm::StringSet<llvm::BumpPtrAllocator> FrameworkNames;
@@ -563,6 +569,12 @@ class HeaderSearch {
getFileInfo(File).DirInfo = SrcMgr::C_System;
}
+ void MarkFileDeprecated(FileEntryRef File, std::string DeprecationMessage) {
+ getFileInfo(File).IsDeprecated = true;
+ DeprecationMessages.emplace_or_assign(&File.getFileEntry(),
+ std::move(DeprecationMessage));
+ }
+
/// Mark the specified file as part of a module.
void MarkFileModuleHeader(FileEntryRef FE, ModuleMap::ModuleHeaderRole Role,
bool isCompilingModuleHeader);
@@ -652,6 +664,13 @@ class HeaderSearch {
std::string getCachedModuleFileName(StringRef ModuleName,
StringRef ModuleMapPath);
+ std::optional<std::string_view>
+ getHeaderDeprecationMessage(FileEntryRef File) {
+ if (!getFileInfo(File).IsDeprecated)
+ return std::nullopt;
+ return DeprecationMessages.at(File);
+ }
+
/// Lookup a module Search for a module with the given name.
///
/// \param ModuleName The name of the module we're looking for.
diff --git a/clang/include/clang/Lex/Preprocessor.h b/clang/include/clang/Lex/Preprocessor.h
index b1c648e647f41..3324996c59554 100644
--- a/clang/include/clang/Lex/Preprocessor.h
+++ b/clang/include/clang/Lex/Preprocessor.h
@@ -2943,6 +2943,7 @@ class Preprocessor {
void HandlePragmaMark(Token &MarkTok);
void HandlePragmaPoison();
void HandlePragmaSystemHeader(Token &SysHeaderTok);
+ void HandlePragmaDeprecatedHeader(Token &Tok, std::string DeprecationMessage);
void HandlePragmaDependency(Token &DependencyTok);
void HandlePragmaPushMacro(Token &Tok);
void HandlePragmaPopMacro(Token &Tok);
diff --git a/clang/lib/Lex/PPDirectives.cpp b/clang/lib/Lex/PPDirectives.cpp
index 891c8ab7f3155..0ea2b412502c2 100644
--- a/clang/lib/Lex/PPDirectives.cpp
+++ b/clang/lib/Lex/PPDirectives.cpp
@@ -2509,6 +2509,15 @@ Preprocessor::ImportAction Preprocessor::HandleHeaderIncludeOrImport(
Action = TrackGMFState.inGMF() ? Import : Skip;
else
Action = (ModuleToImport && !getLangOpts().CompilingPCH) ? Import : Skip;
+
+ // If we're not entering the header, it might have been marked deprecated
+ // the first time it was included.
+ if (auto MaybeMessage = HeaderInfo.getHeaderDeprecationMessage(*File);
+ MaybeMessage) {
+ std::string_view Message = *MaybeMessage;
+ Diag(FilenameTok, diag::warn_pragma_deprecated_header)
+ << !Message.empty() << Message;
+ }
}
// Check for circular inclusion of the main file.
diff --git a/clang/lib/Lex/PPLexerChange.cpp b/clang/lib/Lex/PPLexerChange.cpp
index b014124153c83..52a19d1a3428a 100644
--- a/clang/lib/Lex/PPLexerChange.cpp
+++ b/clang/lib/Lex/PPLexerChange.cpp
@@ -490,6 +490,18 @@ bool Preprocessor::HandleEndOfFile(Token &Result, bool isEndOfMacro) {
SourceMgr.getFileEntryForID(CurPPLexer->getFileID())))
FoundPCHThroughHeader = true;
+ if (CurPPLexer) {
+ if (OptionalFileEntryRef File = CurPPLexer->getFileEntry()) {
+ if (auto MaybeMessage = HeaderInfo.getHeaderDeprecationMessage(*File);
+ MaybeMessage) {
+ std::string_view Message = *MaybeMessage;
+ Diag(SourceMgr.getIncludeLoc(CurPPLexer->getFileID()),
+ diag::warn_pragma_deprecated_header)
+ << !Message.empty() << Message;
+ }
+ }
+ }
+
// We're done with the #included file.
RemoveTopOfLexerStack();
diff --git a/clang/lib/Lex/Pragma.cpp b/clang/lib/Lex/Pragma.cpp
index bba3c89bed38f..47f0974d3aeef 100644
--- a/clang/lib/Lex/Pragma.cpp
+++ b/clang/lib/Lex/Pragma.cpp
@@ -508,6 +508,13 @@ void Preprocessor::HandlePragmaSystemHeader(Token &SysHeaderTok) {
SrcMgr::C_System);
}
+void Preprocessor::HandlePragmaDeprecatedHeader(
+ Token &Tok, std::string DeprecationMessage) {
+ PreprocessorLexer *TheLexer = getCurrentFileLexer();
+ HeaderInfo.MarkFileDeprecated(*TheLexer->getFileEntry(),
+ std::move(DeprecationMessage));
+}
+
/// HandlePragmaDependency - Handle \#pragma GCC dependency "foo" blah.
void Preprocessor::HandlePragmaDependency(Token &DependencyTok) {
Token FilenameTok;
@@ -2084,6 +2091,39 @@ struct PragmaDeprecatedHandler : public PragmaHandler {
}
};
+/// "\#pragma clang deprecated_header"
+struct PragmaDeprecatedHeaderHandler : PragmaHandler {
+ PragmaDeprecatedHeaderHandler() : PragmaHandler("deprecated_header") {}
+
+ void HandlePragma(Preprocessor &PP, PragmaIntroducer Introducer,
+ Token &Tok) override {
+ std::string MessageString;
+
+ PP.Lex(Tok);
+ if (!Tok.is(tok::eod)) {
+ if (!Tok.is(tok::l_paren)) {
+ PP.Diag(Tok, diag::ext_pp_extra_tokens_at_eol)
+ << "pragma clang deprecated_header";
+ } else {
+ PP.Lex(Tok);
+ if (PP.FinishLexStringLiteral(Tok, MessageString,
+ "#pragma clang deprecated_header",
+ /*AllowMacroExpansion=*/true)) {
+ if (Tok.isNot(tok::r_paren)) {
+ PP.Diag(Tok, diag::err_expected) << ")";
+ } else {
+ PP.Lex(Tok);
+ if (!Tok.is(tok::eod))
+ PP.Diag(Tok, diag::ext_pp_extra_tokens_at_eol)
+ << "pragma clang deprecated_header";
+ }
+ }
+ }
+ }
+ PP.HandlePragmaDeprecatedHeader(Tok, MessageString);
+ }
+};
+
/// "\#pragma clang restrict_expansion(...)"
///
/// The syntax is
@@ -2174,6 +2214,7 @@ void Preprocessor::RegisterBuiltinPragmas() {
AddPragmaHandler("clang", new PragmaARCCFCodeAuditedHandler());
AddPragmaHandler("clang", new PragmaAssumeNonNullHandler());
AddPragmaHandler("clang", new PragmaDeprecatedHandler());
+ AddPragmaHandler("clang", new PragmaDeprecatedHeaderHandler());
AddPragmaHandler("clang", new PragmaRestrictExpansionHandler());
AddPragmaHandler("clang", new PragmaFinalHandler());
diff --git a/clang/test/Lexer/deprecated-header-msg.h b/clang/test/Lexer/deprecated-header-msg.h
new file mode 100644
index 0000000000000..6295affba3f7b
--- /dev/null
+++ b/clang/test/Lexer/deprecated-header-msg.h
@@ -0,0 +1,6 @@
+#ifndef DEPRECATED_HEADER_MSG_H
+#define DEPRECATED_HEADER_MSG_H
+
+#pragma clang deprecated_header("This is a shitty header")
+
+#endif // DEPRECATED_HEADER_MSG_H
diff --git a/clang/test/Lexer/deprecated-header.c b/clang/test/Lexer/deprecated-header.c
new file mode 100644
index 0000000000000..caff6b2d09345
--- /dev/null
+++ b/clang/test/Lexer/deprecated-header.c
@@ -0,0 +1,13 @@
+// RUN: %clang_cc1 -Wdeprecated %s -fsyntax-only -verify -Wunknown-pragmas -Wextra-tokens
+
+#pragma clang deprecated_header( // expected-error {{expected string literal in #pragma clang deprecated_header}}
+#pragma clang deprecated_header() // expected-error {{expected string literal in #pragma clang deprecated_header}}
+#pragma clang deprecated_header("" // expected-error {{expected )}}
+#pragma clang deprecated_header something // expected-warning {{extra tokens at end of #pragma clang deprecated_header directive}}
+#pragma clang deprecated_header("") something // expected-warning {{extra tokens at end of #pragma clang deprecated_header directive}}
+
+#include "deprecated-header.h" // expected-warning {{header is deprecated}}
+#include "deprecated-header.h" // expected-warning {{header is deprecated}}
+
+#include "deprecated-header-msg.h" // expected-warning {{header is deprecated: This is a shitty header}}
+#include "deprecated-header-msg.h" // expected-warning {{header is deprecated: This is a shitty header}}
diff --git a/clang/test/Lexer/deprecated-header.h b/clang/test/Lexer/deprecated-header.h
new file mode 100644
index 0000000000000..81884b5630b9c
--- /dev/null
+++ b/clang/test/Lexer/deprecated-header.h
@@ -0,0 +1,6 @@
+#ifndef DEPRECATED_HEADER_H
+#define DEPRECATED_HEADER_H
+
+#pragma clang deprecated_header
+
+#endif // DEPRECATED_HEADER_H
|
AaronBallman
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Overall, I like the idea, but I wonder if a pragma is really the best way forward in 2025. I can imagine standards committees wanting this functionality, and a pragma would likely be just fine in C, but not acceptable in C++.
Did you consider using an attribute instead? e.g., [[clang::deprecated("this header is deprecated")]]; on a null statement in a header file could be a viable approach that both committees would be interested in standardizing under the existing [[deprecated]] attribute.
Some things the PR needs to consider:
- This needs documentation in the language extensions file. In particular, where within a header does the marking show up? Can it be anywhere? Just the "first semantic line" (non-comment, non-header guard)?
- What should happen if the deprecated header ends up including another deprecated header? Normally, deprecated things can reference other deprecated things on the assumption they're all being deprecated together. So perhaps the same is true here? Is it true if deprecated header A includes non-deprecated header B which then includes deprecated header C though?
- We usually suppress diagnostics coming from system headers but this one kind of spans the idea. The system header specifies the diagnostic should happen but the diagnostic itself happens on the inclusion line which may be user code. We need test coverage to make sure user code does get the diagnostic and system headers do not.
- What happens if the marking is used in a module that is imported? I would imagine we'd want the same diagnostic behavior? Perhaps another reason the name should be under
deprecatedrather thandeprecated_header?
| #ifndef DEPRECATED_HEADER_MSG_H | ||
| #define DEPRECATED_HEADER_MSG_H | ||
|
|
||
| #pragma clang deprecated_header("This is a shitty header") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's not acceptable for us to commit.
| #pragma clang deprecated_header() // expected-error {{expected string literal in #pragma clang deprecated_header}} | ||
| #pragma clang deprecated_header("" // expected-error {{expected )}} | ||
| #pragma clang deprecated_header something // expected-warning {{extra tokens at end of #pragma clang deprecated_header directive}} | ||
| #pragma clang deprecated_header("") something // expected-warning {{extra tokens at end of #pragma clang deprecated_header directive}} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should also have a test that we reject writing this in a source file.
This pragma allows marking headers as deprecated, which significantly improves the diagnostics issued compared to the traditional
Specifically, the warning is issued at the place the header is included instead inside the header. This allows suppressing the warning more locally if required. Using the pragma also moves the diagnostic into
-Wdeprecated, which is a much more sensible warning group than-W#warning.-Wno-deprecatedin the command line would suppress the above diagnostic, but we don't tell users.