[clang-tools-extra] [clang-tidy] Fix readability-duplicate-include for includes with macro (PR #87433)
via cfe-commits
cfe-commits at lists.llvm.org
Thu Apr 4 19:16:19 PDT 2024
https://github.com/FruitClover updated https://github.com/llvm/llvm-project/pull/87433
>From aa1c522cbfcfc9313ccfd6868889a9af821e83e2 Mon Sep 17 00:00:00 2001
From: Mike Kashkarov <fruitclover at gmail.com>
Date: Wed, 3 Apr 2024 08:24:00 +0900
Subject: [PATCH 1/4] [clang-tidy] Fix readability-duplicate-include for
includes with macro
Skip hint creation for include directives that form the filename using macros.
---
.../readability/DuplicateIncludeCheck.cpp | 3 ++
.../readability/duplicate-include.cpp | 30 +++++++++++++++++++
2 files changed, 33 insertions(+)
diff --git a/clang-tools-extra/clang-tidy/readability/DuplicateIncludeCheck.cpp b/clang-tools-extra/clang-tidy/readability/DuplicateIncludeCheck.cpp
index 67147164946ab4..7dcba2489e0fe6 100644
--- a/clang-tools-extra/clang-tidy/readability/DuplicateIncludeCheck.cpp
+++ b/clang-tools-extra/clang-tidy/readability/DuplicateIncludeCheck.cpp
@@ -18,6 +18,9 @@ namespace clang::tidy::readability {
static SourceLocation advanceBeyondCurrentLine(const SourceManager &SM,
SourceLocation Start,
int Offset) {
+ // Keep macro location as it is
+ if (Start.isMacroID())
+ return Start;
const FileID Id = SM.getFileID(Start);
const unsigned LineNumber = SM.getSpellingLineNumber(Start);
while (SM.getFileID(Start) == Id &&
diff --git a/clang-tools-extra/test/clang-tidy/checkers/readability/duplicate-include.cpp b/clang-tools-extra/test/clang-tidy/checkers/readability/duplicate-include.cpp
index dd954c705514fb..5ee173bdd27aa2 100644
--- a/clang-tools-extra/test/clang-tidy/checkers/readability/duplicate-include.cpp
+++ b/clang-tools-extra/test/clang-tidy/checkers/readability/duplicate-include.cpp
@@ -70,3 +70,33 @@ int r;
// CHECK-FIXES: {{^int q;$}}
// CHECK-FIXES-NEXT: {{^#include <sys/types.h>$}}
// CHECK-FIXES-NEXT: {{^int r;$}}
+
+namespace Issue_87303 {
+// Include name from macro
+#define MACRO_FILENAME "duplicate-include.h"
+int a;
+#include MACRO_FILENAME
+int b;
+#include "duplicate-include.h"
+int c;
+// CHECK-MESSAGES: :[[@LINE-2]]:1: warning: duplicate include
+// CHECK-FIXES: {{^int a;$}}
+// CHECK-FIXES-NEXT: {{^#include MACRO_FILENAME$}}
+// CHECK-FIXES-NEXT: {{^int b;$}}
+// CHECK-FIXES-NEXT: {{^int c;$}}
+
+// Keep macro
+#define MACRO_FILENAME_2 <duplicate-include2.h>
+int d;
+#include <duplicate-include2.h>
+int e;
+#include MACRO_FILENAME_2
+int f;
+// CHECK-MESSAGES: :[[@LINE-2]]:1: warning: duplicate include
+// CHECK-FIXES: {{^int d;$}}
+// CHECK-FIXES-NEXT: {{^#include <duplicate-include2.h>$}}
+// CHECK-FIXES-NEXT: {{^int e;$}}
+// CHECK-FIXES-NEXT: {{^#include MACRO_FILENAME_2$}}
+// CHECK-FIXES-NEXT: {{^int f;$}}
+
+} // Issue_87303
>From 9735717c8e5314cf8ba642398f2f563370a919d1 Mon Sep 17 00:00:00 2001
From: Mike Kashkarov <fruitclover at gmail.com>
Date: Thu, 4 Apr 2024 12:29:00 +0900
Subject: [PATCH 2/4] [squash] Add release note
---
clang-tools-extra/docs/ReleaseNotes.rst | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst
index 309b844615a121..2e1d749284a522 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -270,6 +270,10 @@ Changes in existing checks
check by resolving fix-it overlaps in template code by disregarding implicit
instances.
+- Improved :doc:`readability-duplicate-include
+ <clang-tidy/checks/readability/duplicate-include>` check by resolving crash
+ at include directives that form the filename using macro.
+
Removed checks
^^^^^^^^^^^^^^
>From 579acdccc8c2db9c9ea92471b69e9c5d8fc351e4 Mon Sep 17 00:00:00 2001
From: Mike Kashkarov <fruitclover at gmail.com>
Date: Thu, 4 Apr 2024 12:48:00 +0900
Subject: [PATCH 3/4] [squash] Fix release note order
---
clang-tools-extra/docs/ReleaseNotes.rst | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst
index 2e1d749284a522..cc4ff6d28fff9f 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -251,6 +251,10 @@ Changes in existing checks
analyzed, se the check now handles the common patterns
`const auto e = (*vector_ptr)[i]` and `const auto e = vector_ptr->at(i);`.
+- Improved :doc:`readability-duplicate-include
+ <clang-tidy/checks/readability/duplicate-include>` check by resolving crash
+ at include directives that form the filename using macro.
+
- Improved :doc:`readability-identifier-naming
<clang-tidy/checks/readability/identifier-naming>` check in `GetConfigPerFile`
mode by resolving symbolic links to header files. Fixed handling of Hungarian
@@ -270,10 +274,6 @@ Changes in existing checks
check by resolving fix-it overlaps in template code by disregarding implicit
instances.
-- Improved :doc:`readability-duplicate-include
- <clang-tidy/checks/readability/duplicate-include>` check by resolving crash
- at include directives that form the filename using macro.
-
Removed checks
^^^^^^^^^^^^^^
>From 8159da2bdcab9ddb9e0a0fb123592dd28ab3d7b3 Mon Sep 17 00:00:00 2001
From: Mike Kashkarov <fruitclover at gmail.com>
Date: Fri, 5 Apr 2024 13:34:15 +0900
Subject: [PATCH 4/4] [squash] Completely skip incudes with macros instead
---
.../readability/DuplicateIncludeCheck.cpp | 7 +++---
clang-tools-extra/docs/ReleaseNotes.rst | 4 ++--
.../readability/duplicate-include.cpp | 23 ++++---------------
3 files changed, 10 insertions(+), 24 deletions(-)
diff --git a/clang-tools-extra/clang-tidy/readability/DuplicateIncludeCheck.cpp b/clang-tools-extra/clang-tidy/readability/DuplicateIncludeCheck.cpp
index 7dcba2489e0fe6..229e5583846b96 100644
--- a/clang-tools-extra/clang-tidy/readability/DuplicateIncludeCheck.cpp
+++ b/clang-tools-extra/clang-tidy/readability/DuplicateIncludeCheck.cpp
@@ -18,9 +18,6 @@ namespace clang::tidy::readability {
static SourceLocation advanceBeyondCurrentLine(const SourceManager &SM,
SourceLocation Start,
int Offset) {
- // Keep macro location as it is
- if (Start.isMacroID())
- return Start;
const FileID Id = SM.getFileID(Start);
const unsigned LineNumber = SM.getSpellingLineNumber(Start);
while (SM.getFileID(Start) == Id &&
@@ -82,6 +79,10 @@ void DuplicateIncludeCallbacks::InclusionDirective(
bool IsAngled, CharSourceRange FilenameRange, OptionalFileEntryRef File,
StringRef SearchPath, StringRef RelativePath, const Module *SuggestedModule,
bool ModuleImported, SrcMgr::CharacteristicKind FileType) {
+ // Skip includes behind macros
+ if (FilenameRange.getBegin().isMacroID() ||
+ FilenameRange.getEnd().isMacroID())
+ return;
if (llvm::is_contained(Files.back(), FileName)) {
// We want to delete the entire line, so make sure that [Start,End] covers
// everything.
diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst
index cc4ff6d28fff9f..9ca0d4767fcf3c 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -252,8 +252,8 @@ Changes in existing checks
`const auto e = (*vector_ptr)[i]` and `const auto e = vector_ptr->at(i);`.
- Improved :doc:`readability-duplicate-include
- <clang-tidy/checks/readability/duplicate-include>` check by resolving crash
- at include directives that form the filename using macro.
+ <clang-tidy/checks/readability/duplicate-include>` check by excluding include
+ directives that form the filename using macro.
- Improved :doc:`readability-identifier-naming
<clang-tidy/checks/readability/identifier-naming>` check in `GetConfigPerFile`
diff --git a/clang-tools-extra/test/clang-tidy/checkers/readability/duplicate-include.cpp b/clang-tools-extra/test/clang-tidy/checkers/readability/duplicate-include.cpp
index 5ee173bdd27aa2..2119602ba454b4 100644
--- a/clang-tools-extra/test/clang-tidy/checkers/readability/duplicate-include.cpp
+++ b/clang-tools-extra/test/clang-tidy/checkers/readability/duplicate-include.cpp
@@ -72,31 +72,16 @@ int r;
// CHECK-FIXES-NEXT: {{^int r;$}}
namespace Issue_87303 {
-// Include name from macro
+#define RESET_INCLUDE_CACHE
+// Expect no warnings
+
#define MACRO_FILENAME "duplicate-include.h"
-int a;
#include MACRO_FILENAME
-int b;
#include "duplicate-include.h"
-int c;
-// CHECK-MESSAGES: :[[@LINE-2]]:1: warning: duplicate include
-// CHECK-FIXES: {{^int a;$}}
-// CHECK-FIXES-NEXT: {{^#include MACRO_FILENAME$}}
-// CHECK-FIXES-NEXT: {{^int b;$}}
-// CHECK-FIXES-NEXT: {{^int c;$}}
-// Keep macro
#define MACRO_FILENAME_2 <duplicate-include2.h>
-int d;
#include <duplicate-include2.h>
-int e;
#include MACRO_FILENAME_2
-int f;
-// CHECK-MESSAGES: :[[@LINE-2]]:1: warning: duplicate include
-// CHECK-FIXES: {{^int d;$}}
-// CHECK-FIXES-NEXT: {{^#include <duplicate-include2.h>$}}
-// CHECK-FIXES-NEXT: {{^int e;$}}
-// CHECK-FIXES-NEXT: {{^#include MACRO_FILENAME_2$}}
-// CHECK-FIXES-NEXT: {{^int f;$}}
+#undef RESET_INCLUDE_CACHE
} // Issue_87303
More information about the cfe-commits
mailing list