Skip to content

[clang-tidy] misc-include-cleaner: redundant insertion on different symbols from same header #65285

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
danix800 opened this issue Sep 5, 2023 · 7 comments · Fixed by #65431
Closed
Assignees

Comments

@danix800
Copy link
Member

danix800 commented Sep 5, 2023

Testcase:

// baz.h

#pragma once
int baz();
#define BAZ 10

// test.c
#include "bar.h"
int BazResult = baz();
int BazResult1 = BAZ;

command:

clang-tidy -fix -checks=-*,misc-include-cleaner test.c

fixed test.c

#include "baz.h"
#include "baz.h"
int BazResult = baz();
int BazResult1 = BAZ;

baz.h is redundant.

@danix800 danix800 changed the title [clang-tidy] misc-include-cleaner: dupliatcate insertion on different symbols from same header [clang-tidy] misc-include-cleaner: redundant insertion on different symbols from same header Sep 5, 2023
@llvmbot
Copy link
Member

llvmbot commented Sep 5, 2023

@llvm/issue-subscribers-clang-tidy

@HerrCai0907
Copy link
Contributor

I cannot reproduce this issue in e22f04b, could you give more detail information about clang-tidy version and system environment.

@danix800
Copy link
Member Author

danix800 commented Sep 5, 2023

One easier way to reproduce is using https://github.com/llvm/llvm-project/blob/e22f04b597e03339739d42e3a60c05634bb74313/clang-tools-extra/test/clang-tidy/checkers/misc/include-cleaner.cpp with this diff:

diff --git a/clang-tools-extra/test/clang-tidy/checkers/misc/Inputs/baz.h b/clang-tools-extra/test/clang-tidy/checkers/misc/Inputs/baz.h
index 042e9ca40401..1961bec3e7b2 100644
--- a/clang-tools-extra/test/clang-tidy/checkers/misc/Inputs/baz.h
+++ b/clang-tools-extra/test/clang-tidy/checkers/misc/Inputs/baz.h
@@ -1,2 +1,3 @@
 #pragma once
 int baz();
+#define BAZ 10
diff --git a/clang-tools-extra/test/clang-tidy/checkers/misc/include-cleaner.cpp b/clang-tools-extra/test/clang-tidy/checkers/misc/include-cleaner.cpp
index e10ac3f46e2e..a1b1c4a3e3a9 100644
--- a/clang-tools-extra/test/clang-tidy/checkers/misc/include-cleaner.cpp
+++ b/clang-tools-extra/test/clang-tidy/checkers/misc/include-cleaner.cpp
@@ -15,3 +15,4 @@ std::string HelloString;
 // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: no header providing "std::string" is directly included [misc-include-cleaner]
 int FooBarResult = foobar();
 // CHECK-MESSAGES: :[[@LINE-1]]:20: warning: no header providing "foobar" is directly included [misc-include-cleaner]
+int BazResult1 = BAZ;

Test with clang-tidy -fix -checks=-*,misc-include-cleaner clang-tools-extra/test/clang-tidy/checkers/misc/include-cleaner.cpp -- -I clang-tools-extra/test/clang-tidy/checkers/misc/Inputs -I clang-tools-extra/test/clang-tidy/checkers/misc/Inputs/system

Produces:

// RUN: %check_clang_tidy %s misc-include-cleaner %t -- -- -I%S/Inputs -isystem%S/Inputs/system
#include "bar.h"
// CHECK-FIXES: {{^}}#include "baz.h"{{$}}
#include "baz.h"
#include "baz.h"
#include "public.h"
// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: included header foo.h is not used directly [misc-include-cleaner]
// CHECK-FIXES: {{^}}
// CHECK-FIXES: {{^}}#include <string>{{$}}
#include <string>
// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: included header vector.h is not used directly [misc-include-cleaner]
// CHECK-FIXES: {{^}}
int BarResult = bar();
int BazResult = baz();
// CHECK-MESSAGES: :[[@LINE-1]]:17: warning: no header providing "baz" is directly included [misc-include-cleaner]
std::string HelloString;
// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: no header providing "std::string" is directly included [misc-include-cleaner]
int FooBarResult = foobar();
// CHECK-MESSAGES: :[[@LINE-1]]:20: warning: no header providing "foobar" is directly included [misc-include-cleaner]
int BazResult1 = BAZ;

@danix800
Copy link
Member Author

danix800 commented Sep 5, 2023

Tested with latest main on linux.

@danix800
Copy link
Member Author

danix800 commented Sep 5, 2023

Or reproduce with randomly picked llvm/Support/FileCollector.cpp, with cmake option
-DCMAKE_EXPORT_COMPILE_COMMANDS=ON so that build/compile_commands.json
is generated, test with

clang-tidy -fix -p build -checks=-*,misc-include-cleaner llvm/lib/Support/FileCollector.cpp

You'll get this

diff --git a/llvm/lib/Support/FileCollector.cpp b/llvm/lib/Support/FileCollector.cpp
index c0ce6b5d74e8..a85bb9b68014 100644
--- a/llvm/lib/Support/FileCollector.cpp
+++ b/llvm/lib/Support/FileCollector.cpp
@@ -7,11 +7,28 @@
 //===----------------------------------------------------------------------===//
 
 #include "llvm/Support/FileCollector.h"
+#include "llvm/ADT/IntrusiveRefCntPtr.h"
 #include "llvm/ADT/SmallString.h"
+#include "llvm/ADT/SmallVector.h"
 #include "llvm/ADT/Twine.h"
+#include "llvm/Support/ErrorOr.h"
 #include "llvm/Support/FileSystem.h"
 #include "llvm/Support/Path.h"
 #include "llvm/Support/Process.h"
+#include <mutex>
+#include <mutex>
+#include <string>
+#include <cassert>
+#include <system_error>
+#include "llvm/Support/VirtualFileSystem.h"
+#include "llvm/Support/VirtualFileSystem.h"
+#include "llvm/Support/VirtualFileSystem.h"
+#include "llvm/Support/raw_ostream.h"
+#include <memory>
+#include <utility>
+#include "llvm/Support/VirtualFileSystem.h"
+#include <memory>
+#include "llvm/Support/VirtualFileSystem.h"
 
 using namespace llvm;

@llvmbot
Copy link
Member

llvmbot commented Sep 5, 2023

@llvm/issue-subscribers-clang-include-cleaner

@danix800 danix800 self-assigned this Sep 5, 2023
@danix800 danix800 assigned HerrCai0907 and unassigned danix800 Sep 6, 2023
HerrCai0907 added a commit that referenced this issue Sep 6, 2023
… multiple times (#65431)

`HeaderIncludes` won't update `ExistingIncludes` during inserting.
We need to manage it in tidy check.

Fixed: #65285
@llvmbot
Copy link
Member

llvmbot commented Sep 6, 2023

@llvm/issue-subscribers-clang-tidy

avillega pushed a commit to avillega/llvm-project that referenced this issue Sep 11, 2023
… multiple times (llvm#65431)

`HeaderIncludes` won't update `ExistingIncludes` during inserting.
We need to manage it in tidy check.

Fixed: llvm#65285
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants