LegalizeAdulthood updated this revision to Diff 402071.
LegalizeAdulthood added a comment.
- Update from review comments
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D7982/new/
https://reviews.llvm.org/D7982
Files:
clang-tools-extra/clang-tidy/readability/CMakeLists.txt
clang-tools-extra/clang-tidy/readability/DuplicateIncludeCheck.cpp
clang-tools-extra/clang-tidy/readability/DuplicateIncludeCheck.h
clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp
clang-tools-extra/docs/ReleaseNotes.rst
clang-tools-extra/docs/clang-tidy/checks/list.rst
clang-tools-extra/docs/clang-tidy/checks/readability-duplicate-include.rst
clang-tools-extra/test/clang-tidy/checkers/Inputs/readability-duplicate-include/readability-duplicate-include.h
clang-tools-extra/test/clang-tidy/checkers/Inputs/readability-duplicate-include/readability-duplicate-include2.h
clang-tools-extra/test/clang-tidy/checkers/Inputs/readability-duplicate-include/system/iostream
clang-tools-extra/test/clang-tidy/checkers/Inputs/readability-duplicate-include/system/string.h
clang-tools-extra/test/clang-tidy/checkers/Inputs/readability-duplicate-include/system/sys/types.h
clang-tools-extra/test/clang-tidy/checkers/Inputs/readability-duplicate-include/system/types.h
clang-tools-extra/test/clang-tidy/checkers/readability-duplicate-include.cpp
Index: clang-tools-extra/test/clang-tidy/checkers/readability-duplicate-include.cpp
===================================================================
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/readability-duplicate-include.cpp
@@ -0,0 +1,72 @@
+// RUN: %check_clang_tidy %s readability-duplicate-include %t -- -- -isystem %S/Inputs/readability-duplicate-include/system -I %S/Inputs/readability-duplicate-include
+
+int a;
+#include <string.h>
+int b;
+#include <string.h>
+int c;
+// CHECK-MESSAGES: :[[@LINE-2]]:1: warning: duplicate include [readability-duplicate-include]
+// CHECK-FIXES: {{^int a;$}}
+// CHECK-FIXES-NEXT: {{^#include <string.h>$}}
+// CHECK-FIXES-NEXT: {{^int b;$}}
+// CHECK-FIXES-NEXT: {{^int c;$}}
+
+int d;
+#include <iostream>
+int e;
+#include <iostream> // extra stuff that will also be removed
+int f;
+// CHECK-MESSAGES: :[[@LINE-2]]:1: warning: duplicate include
+// CHECK-FIXES: {{^int d;$}}
+// CHECK-FIXES-NEXT: {{^#include <iostream>$}}
+// CHECK-FIXES-NEXT: {{^int e;$}}
+// CHECK-FIXES-NEXT: {{^int f;$}}
+
+int g;
+#include "readability-duplicate-include.h"
+int h;
+#include "readability-duplicate-include.h"
+int i;
+// CHECK-MESSAGES: :[[@LINE-2]]:1: warning: duplicate include
+// CHECK-FIXES: {{^int g;$}}
+// CHECK-FIXES-NEXT: {{^#include "readability-duplicate-include.h"$}}
+// CHECK-FIXES-NEXT: {{^int h;$}}
+// CHECK-FIXES-NEXT: {{^int i;$}}
+
+#include <types.h>
+
+int j;
+#include <sys/types.h>
+int k;
+#include <sys/types.h>
+int l;
+// CHECK-MESSAGES: :[[@LINE-2]]:1: warning: duplicate include
+// CHECK-FIXES: {{^int j;$}}
+// CHECK-FIXES-NEXT: {{^#include <sys/types.h>$}}
+// CHECK-FIXES-NEXT: {{^int k;$}}
+// CHECK-FIXES-NEXT: {{^int l;$}}
+
+int m;
+ # include <string.h> // lots of space
+int n;
+// CHECK-MESSAGES: :[[@LINE-2]]:9: warning: duplicate include
+// CHECK-FIXES: {{^int m;$}}
+// CHECK-FIXES-NEXT: {{^int n;$}}
+
+// defining a macro in the main file resets the included file cache
+#define ARBITRARY_MACRO
+int o;
+#include <sys/types.h>
+int p;
+// CHECK-FIXES: {{^int o;$}}
+// CHECK-FIXES-NEXT: {{^#include <sys/types.h>$}}
+// CHECK-FIXES-NEXT: {{^int p;$}}
+
+// undefining a macro resets the cache
+#undef ARBITRARY_MACRO
+int q;
+#include <sys/types.h>
+int r;
+// CHECK-FIXES: {{^int q;$}}
+// CHECK-FIXES-NEXT: {{^#include <sys/types.h>$}}
+// CHECK-FIXES-NEXT: {{^int r;$}}
Index: clang-tools-extra/test/clang-tidy/checkers/Inputs/readability-duplicate-include/system/types.h
===================================================================
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/Inputs/readability-duplicate-include/system/types.h
@@ -0,0 +1 @@
+// This file is intentionally empty.
Index: clang-tools-extra/test/clang-tidy/checkers/Inputs/readability-duplicate-include/system/sys/types.h
===================================================================
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/Inputs/readability-duplicate-include/system/sys/types.h
@@ -0,0 +1 @@
+// This file is intentionally empty.
Index: clang-tools-extra/test/clang-tidy/checkers/Inputs/readability-duplicate-include/system/string.h
===================================================================
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/Inputs/readability-duplicate-include/system/string.h
@@ -0,0 +1 @@
+// This file is intentionally empty.
Index: clang-tools-extra/test/clang-tidy/checkers/Inputs/readability-duplicate-include/system/iostream
===================================================================
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/Inputs/readability-duplicate-include/system/iostream
@@ -0,0 +1 @@
+// This file is intentionally empty.
Index: clang-tools-extra/test/clang-tidy/checkers/Inputs/readability-duplicate-include/readability-duplicate-include2.h
===================================================================
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/Inputs/readability-duplicate-include/readability-duplicate-include2.h
@@ -0,0 +1 @@
+// This file is intentionally empty.
Index: clang-tools-extra/test/clang-tidy/checkers/Inputs/readability-duplicate-include/readability-duplicate-include.h
===================================================================
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/Inputs/readability-duplicate-include/readability-duplicate-include.h
@@ -0,0 +1,15 @@
+#if !defined(READABILITY_DUPLICATE_INCLUDE_H)
+#define READABILITY_DUPLICATE_INCLUDE_H
+
+extern int g;
+#include "readability-duplicate-include2.h"
+extern int h;
+#include "readability-duplicate-include2.h"
+extern int i;
+// CHECK-MESSAGES: :[[@LINE-2]]:1: warning: duplicate include
+// CHECK-FIXES: {{^extern int g;$}}
+// CHECK-FIXES-NEXT: {{^#include "readability-duplicate-include2.h"$}}
+// CHECK-FIXES-NEXT: {{^extern int h;$}}
+// CHECK-FIXES-NEXT: {{^extern int i;$}}
+
+#endif
Index: clang-tools-extra/docs/clang-tidy/checks/readability-duplicate-include.rst
===================================================================
--- /dev/null
+++ clang-tools-extra/docs/clang-tidy/checks/readability-duplicate-include.rst
@@ -0,0 +1,35 @@
+.. title:: clang-tidy - readability-duplicate-include
+
+readability-duplicate-include
+=============================
+
+Looks for duplicate includes and removes them. The check maintains a list of
+included files and looks for duplicates. If a macro is defined or undefined
+then the list of included files is cleared.
+
+Examples:
+
+.. code-block:: c++
+
+ #include <memory>
+ #include <vector>
+ #include <memory>
+
+becomes
+
+.. code-block:: c++
+
+ #include <memory>
+ #include <vector>
+
+Because of the intervening macro definitions, this code remains unchanged:
+
+.. code-block:: c++
+
+ #undef NDEBUG
+ #include "assertion.h"
+ // ...code with assertions enabled
+
+ #define NDEBUG
+ #include "assertion.h"
+ // ...code with assertions disabled
Index: clang-tools-extra/docs/clang-tidy/checks/list.rst
===================================================================
--- clang-tools-extra/docs/clang-tidy/checks/list.rst
+++ clang-tools-extra/docs/clang-tidy/checks/list.rst
@@ -294,6 +294,7 @@
`readability-container-size-empty <readability-container-size-empty.html>`_, "Yes"
`readability-convert-member-functions-to-static <readability-convert-member-functions-to-static.html>`_, "Yes"
`readability-delete-null-pointer <readability-delete-null-pointer.html>`_, "Yes"
+ `readability-duplicate-include <readability-duplicate-include.html>`_, "Yes"
`readability-else-after-return <readability-else-after-return.html>`_, "Yes"
`readability-function-cognitive-complexity <readability-function-cognitive-complexity.html>`_,
`readability-function-size <readability-function-size.html>`_,
Index: clang-tools-extra/docs/ReleaseNotes.rst
===================================================================
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -124,6 +124,11 @@
Finds cases where code could use ``data()`` rather than the address of the
element at index 0 in a container.
+- New :doc:`readability-duplicate-include
+ <clang-tidy/checks/readability-duplicate-include>` check.
+
+ Looks for duplicate includes and removes them.
+
- New :doc:`readability-identifier-length
<clang-tidy/checks/readability-identifier-length>` check.
Index: clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp
===================================================================
--- clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp
+++ clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp
@@ -16,6 +16,7 @@
#include "ContainerSizeEmptyCheck.h"
#include "ConvertMemberFunctionsToStatic.h"
#include "DeleteNullPointerCheck.h"
+#include "DuplicateIncludeCheck.h"
#include "ElseAfterReturnCheck.h"
#include "FunctionCognitiveComplexityCheck.h"
#include "FunctionSizeCheck.h"
@@ -71,6 +72,8 @@
"readability-convert-member-functions-to-static");
CheckFactories.registerCheck<DeleteNullPointerCheck>(
"readability-delete-null-pointer");
+ CheckFactories.registerCheck<DuplicateIncludeCheck>(
+ "readability-duplicate-include");
CheckFactories.registerCheck<ElseAfterReturnCheck>(
"readability-else-after-return");
CheckFactories.registerCheck<FunctionCognitiveComplexityCheck>(
Index: clang-tools-extra/clang-tidy/readability/DuplicateIncludeCheck.h
===================================================================
--- /dev/null
+++ clang-tools-extra/clang-tidy/readability/DuplicateIncludeCheck.h
@@ -0,0 +1,35 @@
+//===--- DuplicateIncludeCheck.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_READABILITY_DUPLICATE_INCLUDE_CHECK_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_DUPLICATE_INCLUDE_CHECK_H
+
+#include "../ClangTidyCheck.h"
+
+namespace clang {
+namespace tidy {
+namespace readability {
+
+/// \brief Find and remove duplicate #include directives.
+///
+/// Only consecutive include directives without any other preprocessor
+/// directives between them are analyzed.
+class DuplicateIncludeCheck : public ClangTidyCheck {
+public:
+ DuplicateIncludeCheck(StringRef Name, ClangTidyContext *Context)
+ : ClangTidyCheck(Name, Context) {}
+
+ void registerPPCallbacks(const SourceManager &SM, Preprocessor *PP,
+ Preprocessor *ModuleExpanderPP) override;
+};
+
+} // namespace readability
+} // namespace tidy
+} // namespace clang
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_DUPLICATE_INCLUDE_CHECK_H
Index: clang-tools-extra/clang-tidy/readability/DuplicateIncludeCheck.cpp
===================================================================
--- /dev/null
+++ clang-tools-extra/clang-tidy/readability/DuplicateIncludeCheck.cpp
@@ -0,0 +1,116 @@
+//===--- DuplicateIncludeCheck.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 "DuplicateIncludeCheck.h"
+#include "clang/Frontend/CompilerInstance.h"
+#include "clang/Lex/Preprocessor.h"
+#include "llvm/ADT/STLExtras.h"
+#include "llvm/ADT/SmallVector.h"
+#include <memory>
+
+namespace clang {
+namespace tidy {
+namespace readability {
+
+static SourceLocation advanceBeyondCurrentLine(const SourceManager &SM,
+ SourceLocation Start,
+ int Offset) {
+ const FileID Id = SM.getFileID(Start);
+ const unsigned LineNumber = SM.getSpellingLineNumber(Start);
+ while (SM.getFileID(Start) == Id &&
+ SM.getSpellingLineNumber(Start.getLocWithOffset(Offset)) == LineNumber)
+ Start = Start.getLocWithOffset(Offset);
+ return Start;
+}
+
+namespace {
+
+using FileList = SmallVector<StringRef>;
+
+class DuplicateIncludeCallbacks : public PPCallbacks {
+public:
+ DuplicateIncludeCallbacks(DuplicateIncludeCheck &Check,
+ const SourceManager &SM)
+ : Check(Check), SM(SM) {
+ // The main file doesn't participate in the FileChanged notification.
+ Files.emplace_back();
+ }
+
+ void FileChanged(SourceLocation Loc, FileChangeReason Reason,
+ SrcMgr::CharacteristicKind FileType,
+ FileID PrevFID) override;
+
+ 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;
+
+ void MacroDefined(const Token &MacroNameTok,
+ const MacroDirective *MD) override;
+
+ void MacroUndefined(const Token &MacroNameTok, const MacroDefinition &MD,
+ const MacroDirective *Undef) override;
+
+private:
+ // A list of included files is kept for each file we enter.
+ SmallVector<FileList> Files;
+ DuplicateIncludeCheck &Check;
+ const SourceManager &SM;
+};
+
+void DuplicateIncludeCallbacks::FileChanged(SourceLocation Loc,
+ FileChangeReason Reason,
+ SrcMgr::CharacteristicKind FileType,
+ FileID PrevFID) {
+ if (Reason == EnterFile)
+ Files.emplace_back();
+ else
+ Files.pop_back();
+}
+
+void DuplicateIncludeCallbacks::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) {
+ if (llvm::find(Files.back(), FileName) != Files.back().end()) {
+ // We want to delete the entire line, so make sure that [Start,End] covers
+ // everything.
+ SourceLocation Start =
+ advanceBeyondCurrentLine(SM, HashLoc, -1).getLocWithOffset(-1);
+ SourceLocation End =
+ advanceBeyondCurrentLine(SM, FilenameRange.getEnd(), 1);
+ Check.diag(HashLoc, "duplicate include")
+ << FixItHint::CreateRemoval(SourceRange{Start, End});
+ } else
+ Files.back().push_back(FileName);
+}
+
+void DuplicateIncludeCallbacks::MacroDefined(const Token &MacroNameTok,
+ const MacroDirective *MD) {
+ Files.back().clear();
+}
+
+void DuplicateIncludeCallbacks::MacroUndefined(const Token &MacroNameTok,
+ const MacroDefinition &MD,
+ const MacroDirective *Undef) {
+ Files.back().clear();
+}
+
+} // namespace
+
+void DuplicateIncludeCheck::registerPPCallbacks(
+ const SourceManager &SM, Preprocessor *PP, Preprocessor *ModuleExpanderPP) {
+ PP->addPPCallbacks(std::make_unique<DuplicateIncludeCallbacks>(*this, SM));
+}
+
+} // namespace readability
+} // namespace tidy
+} // namespace clang
Index: clang-tools-extra/clang-tidy/readability/CMakeLists.txt
===================================================================
--- clang-tools-extra/clang-tidy/readability/CMakeLists.txt
+++ clang-tools-extra/clang-tidy/readability/CMakeLists.txt
@@ -11,6 +11,7 @@
ContainerSizeEmptyCheck.cpp
ConvertMemberFunctionsToStatic.cpp
DeleteNullPointerCheck.cpp
+ DuplicateIncludeCheck.cpp
ElseAfterReturnCheck.cpp
FunctionCognitiveComplexityCheck.cpp
FunctionSizeCheck.cpp
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
