[clang-tools-extra] e8cae48 - Revert "[clang-tidy] modernize-deprecated-headers check should respect extern "C" blocks"
Balazs Benics via cfe-commits
cfe-commits at lists.llvm.org
Fri May 13 08:08:29 PDT 2022
Author: Balazs Benics
Date: 2022-05-13T17:07:58+02:00
New Revision: e8cae487022c2216182ae1ec24f248f287a614b7
URL: https://github.com/llvm/llvm-project/commit/e8cae487022c2216182ae1ec24f248f287a614b7
DIFF: https://github.com/llvm/llvm-project/commit/e8cae487022c2216182ae1ec24f248f287a614b7.diff
LOG: Revert "[clang-tidy] modernize-deprecated-headers check should respect extern "C" blocks"
This reverts commit 7e3ea55da88a9d7feaa22f29d51f89fd0152a189.
Looks like this breaks tests: http://45.33.8.238/linux/76033/step_8.txt
Added:
Modified:
clang-tools-extra/clang-tidy/modernize/DeprecatedHeadersCheck.cpp
clang-tools-extra/clang-tidy/modernize/DeprecatedHeadersCheck.h
clang-tools-extra/docs/ReleaseNotes.rst
Removed:
clang-tools-extra/test/clang-tidy/checkers/Inputs/modernize-deprecated-headers/mylib.h
clang-tools-extra/test/clang-tidy/checkers/modernize-deprecated-headers-extern-c.cpp
################################################################################
diff --git a/clang-tools-extra/clang-tidy/modernize/DeprecatedHeadersCheck.cpp b/clang-tools-extra/clang-tidy/modernize/DeprecatedHeadersCheck.cpp
index dd991221faab..0b44bdaafe23 100644
--- a/clang-tools-extra/clang-tidy/modernize/DeprecatedHeadersCheck.cpp
+++ b/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()));
}
}
diff --git a/clang-tools-extra/clang-tidy/modernize/DeprecatedHeadersCheck.h b/clang-tools-extra/clang-tidy/modernize/DeprecatedHeadersCheck.h
index 1b76ca2ba2bb..113b0ca182e0 100644
--- a/clang-tools-extra/clang-tidy/modernize/DeprecatedHeadersCheck.h
+++ b/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
diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst
index 806e5d10a5fd..00124b0cad75 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/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.
diff --git a/clang-tools-extra/test/clang-tidy/checkers/Inputs/modernize-deprecated-headers/mylib.h b/clang-tools-extra/test/clang-tidy/checkers/Inputs/modernize-deprecated-headers/mylib.h
deleted file mode 100644
index e6adb8fd98e8..000000000000
--- a/clang-tools-extra/test/clang-tidy/checkers/Inputs/modernize-deprecated-headers/mylib.h
+++ /dev/null
@@ -1 +0,0 @@
-#include "assert.h"
diff --git a/clang-tools-extra/test/clang-tidy/checkers/modernize-deprecated-headers-extern-c.cpp b/clang-tools-extra/test/clang-tidy/checkers/modernize-deprecated-headers-extern-c.cpp
deleted file mode 100644
index 344f7ac5a289..000000000000
--- a/clang-tools-extra/test/clang-tidy/checkers/modernize-deprecated-headers-extern-c.cpp
+++ /dev/null
@@ -1,53 +0,0 @@
-// RUN: %check_clang_tidy -std=c++11-or-later %s modernize-deprecated-headers %t \
-// RUN: -- -header-filter='.*' -system-headers \
-// RUN: -extra-arg-before=-isystem%S/Inputs/modernize-deprecated-headers
-
-#define EXTERN_C extern "C"
-
-extern "C++" {
-// We should still have the warnings here.
-#include <stdbool.h>
-// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: including 'stdbool.h' has no effect in C++; consider removing it [modernize-deprecated-headers]
-}
-
-#include <assert.h>
-// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: inclusion of deprecated C++ header 'assert.h'; consider using 'cassert' instead [modernize-deprecated-headers]
-// CHECK-FIXES: {{^}}#include <cassert>{{$}}
-
-#include <stdbool.h>
-// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: including 'stdbool.h' has no effect in C++; consider removing it [modernize-deprecated-headers]
-
-#include <mylib.h>
-// CHECK-MESSAGES: mylib.h:1:10: warning: inclusion of deprecated C++ header 'assert.h'; consider using 'cassert' instead [modernize-deprecated-headers]
-
-namespace wrapping {
-extern "C" {
-#include <assert.h> // no-warning
-#include <mylib.h> // no-warning
-#include <stdbool.h> // no-warning
-}
-} // namespace wrapping
-
-extern "C" {
-namespace wrapped {
-#include <assert.h> // no-warning
-#include <mylib.h> // no-warning
-#include <stdbool.h> // no-warning
-} // namespace wrapped
-}
-
-namespace wrapping {
-extern "C" {
-namespace wrapped {
-#include <assert.h> // no-warning
-#include <mylib.h> // no-warning
-#include <stdbool.h> // no-warning
-} // namespace wrapped
-}
-} // namespace wrapping
-
-EXTERN_C {
-#include <assert.h> // no-warning
-#include <mylib.h> // no-warning
-#include <stdbool.h> // no-warning
-}
More information about the cfe-commits
mailing list