Author: Nico Weber Date: 2020-03-10T10:28:20-04:00 New Revision: 714466bf367dfd5062e1850179d27c37a9ec6b46
URL: https://github.com/llvm/llvm-project/commit/714466bf367dfd5062e1850179d27c37a9ec6b46 DIFF: https://github.com/llvm/llvm-project/commit/714466bf367dfd5062e1850179d27c37a9ec6b46.diff LOG: Revert "[clang-tidy] New check: bugprone-suspicious-include" This reverts commit 1e0669bfe05f0f48ee88152c4a1d581f484f8d67 (and follow-ups 698a12712920c214e39bb215fe26fad2e099425b and 52bbdad7d63fd060d102b3591b433d116a982255). The tests fail fail on Windows, see https://reviews.llvm.org/D74669 Added: Modified: clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt clang-tools-extra/clang-tidy/utils/FileExtensionsUtils.cpp clang-tools-extra/clang-tidy/utils/FileExtensionsUtils.h clang-tools-extra/docs/ReleaseNotes.rst Removed: clang-tools-extra/clang-tidy/bugprone/SuspiciousIncludeCheck.cpp clang-tools-extra/clang-tidy/bugprone/SuspiciousIncludeCheck.h clang-tools-extra/docs/clang-tidy/checks/bugprone-suspicious-include.rst clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/a clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/a.cpp clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/a.hpp clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/c.c clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/c.cc clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/c.cxx clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/i.cpp clang-tools-extra/test/clang-tidy/checkers/bugprone-suspicious-include.cpp ################################################################################ diff --git a/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp b/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp index 9dcb315a257a..86936c678562 100644 --- a/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp +++ b/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp @@ -45,7 +45,6 @@ #include "StringIntegerAssignmentCheck.h" #include "StringLiteralWithEmbeddedNulCheck.h" #include "SuspiciousEnumUsageCheck.h" -#include "SuspiciousIncludeCheck.h" #include "SuspiciousMemsetUsageCheck.h" #include "SuspiciousMissingCommaCheck.h" #include "SuspiciousSemicolonCheck.h" @@ -141,8 +140,6 @@ class BugproneModule : public ClangTidyModule { "bugprone-string-literal-with-embedded-nul"); CheckFactories.registerCheck<SuspiciousEnumUsageCheck>( "bugprone-suspicious-enum-usage"); - CheckFactories.registerCheck<SuspiciousIncludeCheck>( - "bugprone-suspicious-include"); CheckFactories.registerCheck<SuspiciousMemsetUsageCheck>( "bugprone-suspicious-memset-usage"); CheckFactories.registerCheck<SuspiciousMissingCommaCheck>( diff --git a/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt b/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt index a24f3bc7eb0d..c9078ed390d1 100644 --- a/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt +++ b/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt @@ -37,7 +37,6 @@ add_clang_library(clangTidyBugproneModule StringIntegerAssignmentCheck.cpp StringLiteralWithEmbeddedNulCheck.cpp SuspiciousEnumUsageCheck.cpp - SuspiciousIncludeCheck.cpp SuspiciousMemsetUsageCheck.cpp SuspiciousMissingCommaCheck.cpp SuspiciousSemicolonCheck.cpp diff --git a/clang-tools-extra/clang-tidy/bugprone/SuspiciousIncludeCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/SuspiciousIncludeCheck.cpp deleted file mode 100644 index ade7b111fc9a..000000000000 --- a/clang-tools-extra/clang-tidy/bugprone/SuspiciousIncludeCheck.cpp +++ /dev/null @@ -1,105 +0,0 @@ -//===--- SuspiciousIncludeCheck.cpp - clang-tidy --------------------------===// -// -// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. -// See https://llvm.org/LICENSE.txt for license information. -// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception -// -//===----------------------------------------------------------------------===// - -#include "SuspiciousIncludeCheck.h" -#include "clang/AST/ASTContext.h" - -namespace clang { -namespace tidy { -namespace bugprone { - -namespace { -class SuspiciousIncludePPCallbacks : public PPCallbacks { -public: - explicit SuspiciousIncludePPCallbacks(SuspiciousIncludeCheck &Check, - const SourceManager &SM, - Preprocessor *PP) - : Check(Check), PP(PP) {} - - void InclusionDirective(SourceLocation HashLoc, const Token &IncludeTok, - StringRef FileName, bool IsAngled, - CharSourceRange FilenameRange, const FileEntry *File, - StringRef SearchPath, StringRef RelativePath, - const Module *Imported, - SrcMgr::CharacteristicKind FileType) override; - -private: - SuspiciousIncludeCheck &Check; - Preprocessor *PP; -}; -} // namespace - -SuspiciousIncludeCheck::SuspiciousIncludeCheck(StringRef Name, - ClangTidyContext *Context) - : ClangTidyCheck(Name, Context), - RawStringHeaderFileExtensions(Options.getLocalOrGlobal( - "HeaderFileExtensions", utils::defaultHeaderFileExtensions())), - RawStringImplementationFileExtensions(Options.getLocalOrGlobal( - "ImplementationFileExtensions", - utils::defaultImplementationFileExtensions())) { - if (!utils::parseFileExtensions(RawStringImplementationFileExtensions, - ImplementationFileExtensions, - utils::defaultFileExtensionDelimiters())) { - llvm::errs() << "Invalid implementation file extension: " - << RawStringImplementationFileExtensions << "\n"; - } - - if (!utils::parseFileExtensions(RawStringHeaderFileExtensions, - HeaderFileExtensions, - utils::defaultFileExtensionDelimiters())) { - llvm::errs() << "Invalid header file extension: " - << RawStringHeaderFileExtensions << "\n"; - } -} - -void SuspiciousIncludeCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) { - Options.store(Opts, "ImplementationFileExtensions", - RawStringImplementationFileExtensions); - Options.store(Opts, "HeaderFileExtensions", RawStringHeaderFileExtensions); -} - -void SuspiciousIncludeCheck::registerPPCallbacks( - const SourceManager &SM, Preprocessor *PP, Preprocessor *ModuleExpanderPP) { - PP->addPPCallbacks( - ::std::make_unique<SuspiciousIncludePPCallbacks>(*this, SM, PP)); -} - -void SuspiciousIncludePPCallbacks::InclusionDirective( - SourceLocation HashLoc, const Token &IncludeTok, StringRef FileName, - bool IsAngled, CharSourceRange FilenameRange, const FileEntry *File, - StringRef SearchPath, StringRef RelativePath, const Module *Imported, - SrcMgr::CharacteristicKind FileType) { - SourceLocation DiagLoc = FilenameRange.getBegin().getLocWithOffset(1); - - const Optional<StringRef> IFE = - utils::getFileExtension(FileName, Check.ImplementationFileExtensions); - if (!IFE) - return; - - Check.diag(DiagLoc, "suspicious #%0 of file with '%1' extension") - << IncludeTok.getIdentifierInfo()->getName() << *IFE; - - for (const auto &HFE : Check.HeaderFileExtensions) { - SmallString<128> GuessedFileName(FileName); - llvm::sys::path::replace_extension(GuessedFileName, - (HFE.size() ? "." : "") + HFE); - - const DirectoryLookup *CurDir; - Optional<FileEntryRef> File = - PP->LookupFile(DiagLoc, GuessedFileName, IsAngled, nullptr, nullptr, - CurDir, nullptr, nullptr, nullptr, nullptr, nullptr); - if (File) { - Check.diag(DiagLoc, "did you mean to include '%0'?", DiagnosticIDs::Note) - << GuessedFileName; - } - } -} - -} // namespace bugprone -} // namespace tidy -} // namespace clang diff --git a/clang-tools-extra/clang-tidy/bugprone/SuspiciousIncludeCheck.h b/clang-tools-extra/clang-tidy/bugprone/SuspiciousIncludeCheck.h deleted file mode 100644 index 674292da1ee8..000000000000 --- a/clang-tools-extra/clang-tidy/bugprone/SuspiciousIncludeCheck.h +++ /dev/null @@ -1,57 +0,0 @@ -//===--- SuspiciousIncludeCheck.h - clang-tidy ------------------*- C++ -*-===// -// -// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. -// See https://llvm.org/LICENSE.txt for license information. -// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception -// -//===----------------------------------------------------------------------===// - -#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_SUSPICIOUSINCLUDECHECK_H -#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_SUSPICIOUSINCLUDECHECK_H - -#include "../ClangTidyCheck.h" -#include "../utils/FileExtensionsUtils.h" - -namespace clang { -namespace tidy { -namespace bugprone { - -/// Warns on inclusion of files whose names suggest that they're implementation -/// files, instead of headers. E.g: -/// -/// #include "foo.cpp" // warning -/// #include "bar.c" // warning -/// #include "baz.h" // no diagnostic -/// -/// The check supports these options: -/// - `HeaderFileExtensions`: a semicolon-separated list of filename -/// extensions of header files (The filename extensions should not contain -/// "." prefix) ";h;hh;hpp;hxx" by default. For extension-less header -/// files, using an empty string or leaving an empty string between ";" if -/// there are other filename extensions. -/// -/// - `ImplementationFileExtensions`: likewise, a semicolon-separated list of -/// filename extensions of implementation files. "c;cc;cpp;cxx" by default. -/// -/// For the user-facing documentation see: -/// http://clang.llvm.org/extra/clang-tidy/checks/bugprone-suspicious-include.html -class SuspiciousIncludeCheck : public ClangTidyCheck { -public: - SuspiciousIncludeCheck(StringRef Name, ClangTidyContext *Context); - void registerPPCallbacks(const SourceManager &SM, Preprocessor *PP, - Preprocessor *ModuleExpanderPP) override; - void storeOptions(ClangTidyOptions::OptionMap &Opts) override; - - utils::FileExtensionsSet HeaderFileExtensions; - utils::FileExtensionsSet ImplementationFileExtensions; - -private: - const std::string RawStringHeaderFileExtensions; - const std::string RawStringImplementationFileExtensions; -}; - -} // namespace bugprone -} // namespace tidy -} // namespace clang - -#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_SUSPICIOUSINCLUDECHECK_H diff --git a/clang-tools-extra/clang-tidy/utils/FileExtensionsUtils.cpp b/clang-tools-extra/clang-tidy/utils/FileExtensionsUtils.cpp index 4eaf8bc6f392..d6f4b2ae3f7c 100644 --- a/clang-tools-extra/clang-tidy/utils/FileExtensionsUtils.cpp +++ b/clang-tools-extra/clang-tidy/utils/FileExtensionsUtils.cpp @@ -53,20 +53,13 @@ bool parseFileExtensions(StringRef AllFileExtensions, return true; } -llvm::Optional<StringRef> -getFileExtension(StringRef FileName, const FileExtensionsSet &FileExtensions) { +bool isFileExtension(StringRef FileName, + const FileExtensionsSet &FileExtensions) { StringRef Extension = llvm::sys::path::extension(FileName); if (Extension.empty()) - return llvm::None; + return false; // Skip "." prefix. - if (!FileExtensions.count(Extension.substr(1))) - return llvm::None; - return Extension; -} - -bool isFileExtension(StringRef FileName, - const FileExtensionsSet &FileExtensions) { - return getFileExtension(FileName, FileExtensions).hasValue(); + return FileExtensions.count(Extension.substr(1)) > 0; } } // namespace utils diff --git a/clang-tools-extra/clang-tidy/utils/FileExtensionsUtils.h b/clang-tools-extra/clang-tidy/utils/FileExtensionsUtils.h index e5dfae1ba24c..43fc573aad65 100644 --- a/clang-tools-extra/clang-tidy/utils/FileExtensionsUtils.h +++ b/clang-tools-extra/clang-tidy/utils/FileExtensionsUtils.h @@ -11,7 +11,6 @@ #include "clang/Basic/SourceLocation.h" #include "clang/Basic/SourceManager.h" -#include "llvm/ADT/Optional.h" #include "llvm/ADT/SmallSet.h" #include "llvm/ADT/StringRef.h" @@ -37,12 +36,6 @@ bool isSpellingLocInHeaderFile(SourceLocation Loc, SourceManager &SM, /// extensions. inline StringRef defaultHeaderFileExtensions() { return ";h;hh;hpp;hxx"; } -/// Returns recommended default value for the list of implementaiton file -/// extensions. -inline StringRef defaultImplementationFileExtensions() { - return "c;cc;cpp;cxx"; -} - /// Returns recommended default value for the list of file extension /// delimiters. inline StringRef defaultFileExtensionDelimiters() { return ",;"; } @@ -52,11 +45,6 @@ bool parseFileExtensions(StringRef AllFileExtensions, FileExtensionsSet &FileExtensions, StringRef Delimiters); -/// Decides whether a file has a header file extension. -/// Returns the file extension, if included in the provided set. -llvm::Optional<StringRef> -getFileExtension(StringRef FileName, const FileExtensionsSet &FileExtensions); - /// Decides whether a file has one of the specified file extensions. bool isFileExtension(StringRef FileName, const FileExtensionsSet &FileExtensions); diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst index 3da1433934c4..3a58649ea985 100644 --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -82,13 +82,6 @@ New checks Checks for usages of identifiers reserved for use by the implementation. -- New :doc:`bugprone-suspicious-include - <clang-tidy/checks/bugprone-suspicious-include>` check. - - Finds cases where an include refers to what appears to be an implementation - file, which often leads to hard-to-track-down ODR violations, and diagnoses - them. - - New :doc:`cert-oop57-cpp <clang-tidy/checks/cert-oop57-cpp>` check. diff --git a/clang-tools-extra/docs/clang-tidy/checks/bugprone-suspicious-include.rst b/clang-tools-extra/docs/clang-tidy/checks/bugprone-suspicious-include.rst deleted file mode 100644 index 9d6c41da504e..000000000000 --- a/clang-tools-extra/docs/clang-tidy/checks/bugprone-suspicious-include.rst +++ /dev/null @@ -1,33 +0,0 @@ -.. title:: clang-tidy - bugprone-suspicious-include - -bugprone-suspicious-include -=========================== - -The check detects various cases when an include refers to what appears to be an -implementation file, which often leads to hard-to-track-down ODR violations. - -Examples: - -.. code-block:: c++ - - #include "Dinosaur.hpp" // OK, .hpp files tend not to have definitions. - #include "Pterodactyl.h" // OK, .h files tend not to have definitions. - #include "Velociraptor.cpp" // Warning, filename is suspicious. - #import "Stegosaurus.c" // Warning, fliename is suspicious. - #include_next <stdio.c> // Warning, filename is suspicious. - -Options -------- -.. option:: HeaderFileExtensions - - Default value: `";h;hh;hpp;hxx"` - A semicolon-separated list of filename extensions of header files (the - filename extensions should not contain a "." prefix). For extension-less - header files, use an empty string or leave an empty string between ";" - if there are other filename extensions. - -.. option:: ImplementationFileExtensions - - Default value: `"c;cc;cpp;cxx"` - Likewise, a semicolon-separated list of filename extensions of - implementation files. diff --git a/clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/a b/clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/a deleted file mode 100644 index e69de29bb2d1..000000000000 diff --git a/clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/a.cpp b/clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/a.cpp deleted file mode 100644 index e69de29bb2d1..000000000000 diff --git a/clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/a.hpp b/clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/a.hpp deleted file mode 100644 index e69de29bb2d1..000000000000 diff --git a/clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/c.c b/clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/c.c deleted file mode 100644 index e69de29bb2d1..000000000000 diff --git a/clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/c.cc b/clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/c.cc deleted file mode 100644 index e69de29bb2d1..000000000000 diff --git a/clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/c.cxx b/clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/c.cxx deleted file mode 100644 index e69de29bb2d1..000000000000 diff --git a/clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/i.cpp b/clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/i.cpp deleted file mode 100644 index e69de29bb2d1..000000000000 diff --git a/clang-tools-extra/test/clang-tidy/checkers/bugprone-suspicious-include.cpp b/clang-tools-extra/test/clang-tidy/checkers/bugprone-suspicious-include.cpp deleted file mode 100644 index 72d1e3b70f80..000000000000 --- a/clang-tools-extra/test/clang-tidy/checkers/bugprone-suspicious-include.cpp +++ /dev/null @@ -1,27 +0,0 @@ -// RUN: %check_clang_tidy %s bugprone-suspicious-include %t -- -- -isystem %S/Inputs/Headers -fmodules - -// clang-format off - -// CHECK-MESSAGES: [[@LINE+4]]:11: warning: suspicious #include of file with '.cpp' extension -// CHECK-MESSAGES: [[@LINE+3]]:11: note: did you mean to include 'a'? -// CHECK-MESSAGES: [[@LINE+2]]:11: note: did you mean to include 'a.h'? -// CHECK-MESSAGES: [[@LINE+1]]:11: note: did you mean to include 'a.hpp'? -#include "a.cpp" - -// CHECK-MESSAGES: [[@LINE+2]]:11: warning: suspicious #include of file with '.cpp' extension -// CHECK-MESSAGES: [[@LINE+1]]:11: note: did you mean to include 'i.h'? -#include "i.cpp" - -#include "b.h" - -// CHECK-MESSAGES: [[@LINE+1]]:10: warning: suspicious #import of file with '.c' extension -#import "c.c" - -// CHECK-MESSAGES: [[@LINE+1]]:16: warning: suspicious #include_next of file with '.c' extension -#include_next <c.c> - -// CHECK-MESSAGES: [[@LINE+1]]:13: warning: suspicious #include of file with '.cc' extension -# include <c.cc> - -// CHECK-MESSAGES: [[@LINE+1]]:14: warning: suspicious #include of file with '.cxx' extension -# include <c.cxx> _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
