Skip to content

[clang-scan-deps] Remove const from ModuleDeps loop to enable move.#161109

Merged
naveen-seth merged 1 commit into
llvm:mainfrom
naveen-seth:scan-deps-move-prevented-by-const
Sep 29, 2025
Merged

[clang-scan-deps] Remove const from ModuleDeps loop to enable move.#161109
naveen-seth merged 1 commit into
llvm:mainfrom
naveen-seth:scan-deps-move-prevented-by-const

Conversation

@naveen-seth

Copy link
Copy Markdown
Contributor

This changes the iteration from const to non-const so that std::move results in a true move rather than a copy.

This changes the iteration from const to non-const so that std::move
results in a true move rather than a copy.
@llvmbot llvmbot added the clang Clang issues not falling into any other category label Sep 29, 2025
@naveen-seth naveen-seth added the clang:modules C++20 modules and Clang Header Modules label Sep 29, 2025
@llvmbot

llvmbot commented Sep 29, 2025

Copy link
Copy Markdown
Member

@llvm/pr-subscribers-clang-modules

@llvm/pr-subscribers-clang

Author: Naveen Seth Hanig (naveen-seth)

Changes

This changes the iteration from const to non-const so that std::move results in a true move rather than a copy.


Full diff: https://github.com/llvm/llvm-project/pull/161109.diff

1 Files Affected:

  • (modified) clang/tools/clang-scan-deps/ClangScanDeps.cpp (+1-1)
diff --git a/clang/tools/clang-scan-deps/ClangScanDeps.cpp b/clang/tools/clang-scan-deps/ClangScanDeps.cpp
index 0e2758d123edc..e41f4eb7999ae 100644
--- a/clang/tools/clang-scan-deps/ClangScanDeps.cpp
+++ b/clang/tools/clang-scan-deps/ClangScanDeps.cpp
@@ -420,7 +420,7 @@ class FullDeps {
     std::vector<ModuleDeps *> NewMDs;
     {
       std::unique_lock<std::mutex> ul(Lock);
-      for (const ModuleDeps &MD : Graph) {
+      for (ModuleDeps &MD : Graph) {
         auto I = Modules.find({MD.ID, 0});
         if (I != Modules.end()) {
           I->first.InputIndex = std::min(I->first.InputIndex, InputIndex);

@ChuanqiXu9 ChuanqiXu9 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is this a standalone PR? I feel this can be within other PR. The PR itself doesn't do anything.

@naveen-seth

Copy link
Copy Markdown
Contributor Author

Ah I see, I've moved it to #155523 now (even though that is also NFC). Hope that’s fine anyway.
Otherwise I can add them to a more meaningful PR in the future.

@Bigcheese

Copy link
Copy Markdown
Contributor

This PR does do something, it change a copy to a move of a large data structure, which has (minor) perf/memory impact. This seems fine to me as a standalone PR, and doesn't really fit as a side change in any other existing PR.

@Bigcheese Bigcheese reopened this Sep 29, 2025
@naveen-seth naveen-seth merged commit d28c07b into llvm:main Sep 29, 2025
23 checks passed
mahesh-attarde pushed a commit to mahesh-attarde/llvm-project that referenced this pull request Oct 3, 2025
…lvm#161109)

This changes the iteration from const to non-const so that std::move
results in a true move rather than a copy.
@naveen-seth naveen-seth deleted the scan-deps-move-prevented-by-const branch October 20, 2025 14:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

clang:modules C++20 modules and Clang Header Modules clang Clang issues not falling into any other category

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants