Revert "[clang-tidy] modernize-deprecated-headers check should respect extern "C" blocks"

This reverts commit 7e3ea55.

Looks like this breaks tests: http://45.33.8.238/linux/76033/step_8.txt

Author: Balazs Benics

clang-tools-extra/clang-tidy/modernize/DeprecatedHeadersCheck.cpp

@@ -7,37 +7,23 @@
 //===----------------------------------------------------------------------===//
 
 #include "DeprecatedHeadersCheck.h"
-#include "clang/AST/RecursiveASTVisitor.h"
 #include "clang/Frontend/CompilerInstance.h"
 #include "clang/Lex/PPCallbacks.h"
 #include "clang/Lex/Preprocessor.h"
 #include "llvm/ADT/StringMap.h"
 #include "llvm/ADT/StringSet.h"
 
-#include <algorithm>
 #include <vector>
 
 namespace clang {
 namespace tidy {
 namespace modernize {
-namespace detail {
-bool operator<(const IncludeMarker &LHS, const IncludeMarker &RHS) {
-  return LHS.DecomposedDiagLoc < RHS.DecomposedDiagLoc;
-}
-bool operator<(const IncludeMarker &LHS,
-               const std::pair<FileID, unsigned> &RHS) {
-  return LHS.DecomposedDiagLoc < RHS;
-}
-bool operator<(const std::pair<FileID, unsigned> &LHS,
-               const IncludeMarker &RHS) {
-  return LHS < RHS.DecomposedDiagLoc;
-}
 
+namespace {
 class IncludeModernizePPCallbacks : public PPCallbacks {
 public:
-  explicit IncludeModernizePPCallbacks(DeprecatedHeadersCheck &Check,
-                                       LangOptions LangOpts,
-                                       const SourceManager &SM);
+  explicit IncludeModernizePPCallbacks(ClangTidyCheck &Check,
+                                       LangOptions LangOpts);
 
   void InclusionDirective(SourceLocation HashLoc, const Token &IncludeTok,
                           StringRef FileName, bool IsAngled,
@@ -47,98 +33,22 @@ class IncludeModernizePPCallbacks : public PPCallbacks {
                           SrcMgr::CharacteristicKind FileType) override;
 
 private:
-  DeprecatedHeadersCheck &Check;
+  ClangTidyCheck &Check;
   LangOptions LangOpts;
   llvm::StringMap<std::string> CStyledHeaderToCxx;
   llvm::StringSet<> DeleteHeaders;
-  const SourceManager &SM;
-};
-
-class ExternCRefutationVisitor
-    : public RecursiveASTVisitor<ExternCRefutationVisitor> {
-  std::vector<IncludeMarker> &IncludesToBeProcessed;
-  const SourceManager &SM;
-
-public:
-  ExternCRefutationVisitor(std::vector<IncludeMarker> &IncludesToBeProcessed,
-                           SourceManager &SM)
-      : IncludesToBeProcessed(IncludesToBeProcessed), SM(SM) {}
-  bool shouldWalkTypesOfTypeLocs() const { return false; }
-  bool shouldVisitLambdaBody() const { return false; }
-
-  bool VisitLinkageSpecDecl(LinkageSpecDecl *LinkSpecDecl) const {
-    if (LinkSpecDecl->getLanguage() != LinkageSpecDecl::lang_c ||
-        !LinkSpecDecl->hasBraces())
-      return true;
-
-    auto ExternCBlockBegin =
-        SM.getDecomposedExpansionLoc(LinkSpecDecl->getBeginLoc());
-    auto ExternCBlockEnd =
-        SM.getDecomposedExpansionLoc(LinkSpecDecl->getEndLoc());
-
-    auto Begin = IncludesToBeProcessed.begin();
-    auto End = IncludesToBeProcessed.end();
-    auto Low = std::lower_bound(Begin, End, ExternCBlockBegin);
-    auto Up = std::upper_bound(Low, End, ExternCBlockEnd);
-    IncludesToBeProcessed.erase(Low, Up);
-    return true;
-  }
 };
-} // namespace detail
+} // namespace
 
 void DeprecatedHeadersCheck::registerPPCallbacks(
     const SourceManager &SM, Preprocessor *PP, Preprocessor *ModuleExpanderPP) {
-  PP->addPPCallbacks(::std::make_unique<detail::IncludeModernizePPCallbacks>(
-      *this, getLangOpts(), PP->getSourceManager()));
-}
-void DeprecatedHeadersCheck::registerMatchers(
-    ast_matchers::MatchFinder *Finder) {
-  // Even though the checker operates on a "preprocessor" level, we still need
-  // to act on a "TranslationUnit" to acquire the AST where we can walk each
-  // Decl and look for `extern "C"` blocks where we will suppress the report we
-  // collected during the preprocessing phase.
-  // The `onStartOfTranslationUnit()` won't suffice, since we need some handle
-  // to the `ASTContext`.
-  Finder->addMatcher(ast_matchers::translationUnitDecl().bind("TU"), this);
-}
-
-void DeprecatedHeadersCheck::onEndOfTranslationUnit() {
-  IncludesToBeProcessed.clear();
-}
-
-void DeprecatedHeadersCheck::check(
-    const ast_matchers::MatchFinder::MatchResult &Result) {
-  SourceManager &SM = Result.Context->getSourceManager();
-  using detail::IncludeMarker;
-
-  llvm::sort(IncludesToBeProcessed);
-
-  // Suppress includes wrapped by `extern "C" { ... }` blocks.
-  detail::ExternCRefutationVisitor Visitor(IncludesToBeProcessed, SM);
-  Visitor.TraverseAST(*Result.Context);
-
-  // Emit all the remaining reports.
-  for (const IncludeMarker &Entry : IncludesToBeProcessed) {
-    SourceLocation DiagLoc = SM.getComposedLoc(Entry.DecomposedDiagLoc.first,
-                                               Entry.DecomposedDiagLoc.second);
-    if (Entry.Replacement.empty()) {
-      diag(DiagLoc, "including '%0' has no effect in C++; consider removing it")
-          << Entry.FileName << FixItHint::CreateRemoval(Entry.ReplacementRange);
-    } else {
-      diag(DiagLoc, "inclusion of deprecated C++ header "
-                    "'%0'; consider using '%1' instead")
-          << Entry.FileName << Entry.Replacement
-          << FixItHint::CreateReplacement(
-                 Entry.ReplacementRange,
-                 (llvm::Twine("<") + Entry.Replacement + ">").str());
-    }
-  }
+    PP->addPPCallbacks(
+        ::std::make_unique<IncludeModernizePPCallbacks>(*this, getLangOpts()));
 }
 
-detail::IncludeModernizePPCallbacks::IncludeModernizePPCallbacks(
-    DeprecatedHeadersCheck &Check, LangOptions LangOpts,
-    const SourceManager &SM)
-    : Check(Check), LangOpts(LangOpts), SM(SM) {
+IncludeModernizePPCallbacks::IncludeModernizePPCallbacks(ClangTidyCheck &Check,
+                                                         LangOptions LangOpts)
+    : Check(Check), LangOpts(LangOpts) {
   for (const auto &KeyValue :
        std::vector<std::pair<llvm::StringRef, std::string>>(
            {{"assert.h", "cassert"},
@@ -179,7 +89,7 @@ detail::IncludeModernizePPCallbacks::IncludeModernizePPCallbacks(
   }
 }
 
-void detail::IncludeModernizePPCallbacks::InclusionDirective(
+void IncludeModernizePPCallbacks::InclusionDirective(
     SourceLocation HashLoc, const Token &IncludeTok, StringRef FileName,
     bool IsAngled, CharSourceRange FilenameRange, Optional<FileEntryRef> File,
     StringRef SearchPath, StringRef RelativePath, const Module *Imported,
@@ -191,16 +101,19 @@ void detail::IncludeModernizePPCallbacks::InclusionDirective(
   // 1. Insert std prefix for every such symbol occurrence.
   // 2. Insert `using namespace std;` to the beginning of TU.
   // 3. Do nothing and let the user deal with the migration himself.
-  std::pair<FileID, unsigned> DiagLoc =
-      SM.getDecomposedExpansionLoc(FilenameRange.getBegin());
   if (CStyledHeaderToCxx.count(FileName) != 0) {
-    Check.IncludesToBeProcessed.push_back(
-        IncludeMarker{CStyledHeaderToCxx[FileName], FileName,
-                      FilenameRange.getAsRange(), DiagLoc});
+    std::string Replacement =
+        (llvm::Twine("<") + CStyledHeaderToCxx[FileName] + ">").str();
+    Check.diag(FilenameRange.getBegin(), "inclusion of deprecated C++ header "
+                                         "'%0'; consider using '%1' instead")
+        << FileName << CStyledHeaderToCxx[FileName]
+        << FixItHint::CreateReplacement(FilenameRange.getAsRange(),
+                                        Replacement);
   } else if (DeleteHeaders.count(FileName) != 0) {
-    Check.IncludesToBeProcessed.push_back(
-        IncludeMarker{std::string{}, FileName,
-                      SourceRange{HashLoc, FilenameRange.getEnd()}, DiagLoc});
+    Check.diag(FilenameRange.getBegin(),
+               "including '%0' has no effect in C++; consider removing it")
+        << FileName << FixItHint::CreateRemoval(
+                           SourceRange(HashLoc, FilenameRange.getEnd()));
   }
 }
 

clang-tools-extra/clang-tidy/modernize/DeprecatedHeadersCheck.h

@@ -15,22 +15,6 @@ namespace clang {
 namespace tidy {
 namespace modernize {
 
-namespace detail {
-class IncludeModernizePPCallbacks;
-class ExternCRefutationVisitor;
-struct IncludeMarker {
-  std::string Replacement;
-  StringRef FileName;
-  SourceRange ReplacementRange;
-  std::pair<FileID, unsigned> DecomposedDiagLoc;
-};
-bool operator<(const IncludeMarker &LHS, const IncludeMarker &RHS);
-bool operator<(const IncludeMarker &LHS,
-               const std::pair<FileID, unsigned> &RHS);
-bool operator<(const std::pair<FileID, unsigned> &LHS,
-               const IncludeMarker &RHS);
-} // namespace detail
-
 /// This check replaces deprecated C library headers with their C++ STL
 /// alternatives.
 ///
@@ -57,14 +41,6 @@ class DeprecatedHeadersCheck : public ClangTidyCheck {
   }
   void registerPPCallbacks(const SourceManager &SM, Preprocessor *PP,
                            Preprocessor *ModuleExpanderPP) override;
-  void registerMatchers(ast_matchers::MatchFinder *Finder) override;
-  void onEndOfTranslationUnit() override;
-  void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
-
-private:
-  friend class detail::IncludeModernizePPCallbacks;
-  friend class detail::ExternCRefutationVisitor;
-  std::vector<detail::IncludeMarker> IncludesToBeProcessed;
 };
 
 } // namespace modernize

clang-tools-extra/docs/ReleaseNotes.rst

@@ -170,11 +170,6 @@ Changes in existing checks
   <clang-tidy/checks/misc-redundant-expression>` involving assignments in
   conditions. This fixes `Issue 35853 <https://github.com/llvm/llvm-project/issues/35853>`_.
 
-- Fixed a false positive in :doc:`modernize-deprecated-headers
-  <clang-tidy/checks/modernize-deprecated-headers>` involving including
-  C header files from C++ files wrapped by ``extern "C" { ... }`` blocks.
-  Such includes will be ignored by now.
-
 - Improved :doc:`performance-inefficient-vector-operation
   <clang-tidy/checks/performance-inefficient-vector-operation>` to work when
   the vector is a member of a structure.