Skip to content

Conversation

localspook
Copy link
Contributor

@localspook localspook commented Sep 26, 2025

  • IncludeModernizePPCallbacks creates temporary vectors when all it needs is constant arrays
  • The header replacement strings are std::string when they can just be StringRef
  • IncludeModernizePPCallbacks's constructor
    1. Takes LangOptions by value (the thing is 832 bytes)
    2. Stores it, but none of IncludeModernizePPCallbacks's member functions use it

@llvmbot
Copy link
Member

llvmbot commented Sep 26, 2025

@llvm/pr-subscribers-clang-tidy

@llvm/pr-subscribers-clang-tools-extra

Author: Victor Chernyakin (localspook)

Changes
  • IncludeModernizePPCallbacks creates a temporary vector when all it needs is a constant array
  • The header replacement strings are std::string when they can just be StringRef
  • IncludeModernizePPCallbacks's constructor
    1. Takes LangOptions by value (the thing is 832 bytes)
    2. Stores it, but none of IncludeModernizePPCallbacks's member functions use it

Full diff: https://github.com/llvm/llvm-project/pull/160967.diff

2 Files Affected:

  • (modified) clang-tools-extra/clang-tidy/modernize/DeprecatedHeadersCheck.cpp (+33-43)
  • (modified) clang-tools-extra/clang-tidy/modernize/DeprecatedHeadersCheck.h (+1-1)
diff --git a/clang-tools-extra/clang-tidy/modernize/DeprecatedHeadersCheck.cpp b/clang-tools-extra/clang-tidy/modernize/DeprecatedHeadersCheck.cpp
index 9f4c215614287..01aa9feb27e57 100644
--- a/clang-tools-extra/clang-tidy/modernize/DeprecatedHeadersCheck.cpp
+++ b/clang-tools-extra/clang-tidy/modernize/DeprecatedHeadersCheck.cpp
@@ -24,8 +24,9 @@ namespace {
 class IncludeModernizePPCallbacks : public PPCallbacks {
 public:
   explicit IncludeModernizePPCallbacks(
-      std::vector<IncludeMarker> &IncludesToBeProcessed, LangOptions LangOpts,
-      const SourceManager &SM, bool CheckHeaderFile);
+      std::vector<IncludeMarker> &IncludesToBeProcessed,
+      const LangOptions &LangOpts, const SourceManager &SM,
+      bool CheckHeaderFile);
 
   void InclusionDirective(SourceLocation HashLoc, const Token &IncludeTok,
                           StringRef FileName, bool IsAngled,
@@ -37,8 +38,7 @@ class IncludeModernizePPCallbacks : public PPCallbacks {
 
 private:
   std::vector<IncludeMarker> &IncludesToBeProcessed;
-  LangOptions LangOpts;
-  llvm::StringMap<std::string> CStyledHeaderToCxx;
+  llvm::StringMap<StringRef> CStyledHeaderToCxx;
   llvm::StringSet<> DeleteHeaders;
   const SourceManager &SM;
   bool CheckHeaderFile;
@@ -131,48 +131,38 @@ void DeprecatedHeadersCheck::check(
 }
 
 IncludeModernizePPCallbacks::IncludeModernizePPCallbacks(
-    std::vector<IncludeMarker> &IncludesToBeProcessed, LangOptions LangOpts,
-    const SourceManager &SM, bool CheckHeaderFile)
-    : IncludesToBeProcessed(IncludesToBeProcessed), LangOpts(LangOpts), SM(SM),
+    std::vector<IncludeMarker> &IncludesToBeProcessed,
+    const LangOptions &LangOpts, const SourceManager &SM, bool CheckHeaderFile)
+    : IncludesToBeProcessed(IncludesToBeProcessed), SM(SM),
       CheckHeaderFile(CheckHeaderFile) {
-  for (const auto &KeyValue :
-       std::vector<std::pair<llvm::StringRef, std::string>>(
-           {{"assert.h", "cassert"},
-            {"complex.h", "complex"},
-            {"ctype.h", "cctype"},
-            {"errno.h", "cerrno"},
-            {"float.h", "cfloat"},
-            {"limits.h", "climits"},
-            {"locale.h", "clocale"},
-            {"math.h", "cmath"},
-            {"setjmp.h", "csetjmp"},
-            {"signal.h", "csignal"},
-            {"stdarg.h", "cstdarg"},
-            {"stddef.h", "cstddef"},
-            {"stdio.h", "cstdio"},
-            {"stdlib.h", "cstdlib"},
-            {"string.h", "cstring"},
-            {"time.h", "ctime"},
-            {"wchar.h", "cwchar"},
-            {"wctype.h", "cwctype"}})) {
+
+  static constexpr std::pair<StringRef, StringRef> CXX98Headers[] = {
+      {"assert.h", "cassert"}, {"complex.h", "complex"},
+      {"ctype.h", "cctype"},   {"errno.h", "cerrno"},
+      {"float.h", "cfloat"},   {"limits.h", "climits"},
+      {"locale.h", "clocale"}, {"math.h", "cmath"},
+      {"setjmp.h", "csetjmp"}, {"signal.h", "csignal"},
+      {"stdarg.h", "cstdarg"}, {"stddef.h", "cstddef"},
+      {"stdio.h", "cstdio"},   {"stdlib.h", "cstdlib"},
+      {"string.h", "cstring"}, {"time.h", "ctime"},
+      {"wchar.h", "cwchar"},   {"wctype.h", "cwctype"},
+  };
+  for (const auto &KeyValue : CXX98Headers)
     CStyledHeaderToCxx.insert(KeyValue);
-  }
-  // Add C++11 headers.
-  if (LangOpts.CPlusPlus11) {
-    for (const auto &KeyValue :
-         std::vector<std::pair<llvm::StringRef, std::string>>(
-             {{"fenv.h", "cfenv"},
-              {"stdint.h", "cstdint"},
-              {"inttypes.h", "cinttypes"},
-              {"tgmath.h", "ctgmath"},
-              {"uchar.h", "cuchar"}})) {
+
+  static constexpr std::pair<StringRef, StringRef> CXX11Headers[] = {
+      {"fenv.h", "cfenv"},         {"stdint.h", "cstdint"},
+      {"inttypes.h", "cinttypes"}, {"tgmath.h", "ctgmath"},
+      {"uchar.h", "cuchar"},
+  };
+  if (LangOpts.CPlusPlus11)
+    for (const auto &KeyValue : CXX11Headers)
       CStyledHeaderToCxx.insert(KeyValue);
-    }
-  }
-  for (const auto &Key :
-       std::vector<std::string>({"stdalign.h", "stdbool.h", "iso646.h"})) {
+
+  static constexpr StringRef HeadersToDelete[] = {"stdalign.h", "stdbool.h",
+                                                  "iso646.h"};
+  for (const auto &Key : HeadersToDelete)
     DeleteHeaders.insert(Key);
-  }
 }
 
 void IncludeModernizePPCallbacks::InclusionDirective(
@@ -205,7 +195,7 @@ void IncludeModernizePPCallbacks::InclusionDirective(
   } else if (DeleteHeaders.contains(FileName)) {
     IncludesToBeProcessed.emplace_back(
         // NOLINTNEXTLINE(modernize-use-emplace) - false-positive
-        IncludeMarker{std::string{}, FileName,
+        IncludeMarker{StringRef{}, FileName,
                       SourceRange{HashLoc, FilenameRange.getEnd()}, DiagLoc});
   }
 }
diff --git a/clang-tools-extra/clang-tidy/modernize/DeprecatedHeadersCheck.h b/clang-tools-extra/clang-tidy/modernize/DeprecatedHeadersCheck.h
index c9409cb641c54..03cf433aa2809 100644
--- a/clang-tools-extra/clang-tidy/modernize/DeprecatedHeadersCheck.h
+++ b/clang-tools-extra/clang-tidy/modernize/DeprecatedHeadersCheck.h
@@ -44,7 +44,7 @@ class DeprecatedHeadersCheck : public ClangTidyCheck {
   void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
 
   struct IncludeMarker {
-    std::string Replacement;
+    StringRef Replacement;
     StringRef FileName;
     SourceRange ReplacementRange;
     SourceLocation DiagLoc;

@localspook localspook merged commit 233d122 into llvm:main Oct 9, 2025
10 checks passed
@localspook localspook deleted the deprecated-headers branch October 9, 2025 04:23
svkeerthy pushed a commit that referenced this pull request Oct 9, 2025
…eaders` (#160967)

- `IncludeModernizePPCallbacks` creates temporary vectors when all it
needs is constant arrays
- The header replacement strings are `std::string` when they can just be
`StringRef`
- `IncludeModernizePPCallbacks`'s constructor
  1. Takes `LangOptions` by value (the thing is 832 bytes)
2. Stores it, but none of `IncludeModernizePPCallbacks`'s member
functions use it
clingfei pushed a commit to clingfei/llvm-project that referenced this pull request Oct 10, 2025
…eaders` (llvm#160967)

- `IncludeModernizePPCallbacks` creates temporary vectors when all it
needs is constant arrays
- The header replacement strings are `std::string` when they can just be
`StringRef`
- `IncludeModernizePPCallbacks`'s constructor
  1. Takes `LangOptions` by value (the thing is 832 bytes)
2. Stores it, but none of `IncludeModernizePPCallbacks`'s member
functions use it
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants