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 <fruitclo...@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 <fruitclo...@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 <fruitclo...@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 <fruitclo...@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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits