Skip to content

Conversation

shafik
Copy link
Collaborator

@shafik shafik commented Aug 15, 2025

Static analysis flagged this code because it will cause unneeded copies of std::string. This fix is merely to use const auto & in the trailing return type.

…tion

Static analysis flagged this code because it will cause unneeded copies of
std::string. This fix is merely to use const auto & in the trailing return type.
@llvmbot llvmbot added the clang Clang issues not falling into any other category label Aug 15, 2025
@llvmbot
Copy link
Member

llvmbot commented Aug 15, 2025

@llvm/pr-subscribers-clang

Author: Shafik Yaghmour (shafik)

Changes

Static analysis flagged this code because it will cause unneeded copies of std::string. This fix is merely to use const auto & in the trailing return type.


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

1 Files Affected:

  • (modified) clang/include/clang/Tooling/DependencyScanning/DependencyScanningTool.h (+1-1)
diff --git a/clang/include/clang/Tooling/DependencyScanning/DependencyScanningTool.h b/clang/include/clang/Tooling/DependencyScanning/DependencyScanningTool.h
index c3601a4e73e1f..81177a04654bb 100644
--- a/clang/include/clang/Tooling/DependencyScanning/DependencyScanningTool.h
+++ b/clang/include/clang/Tooling/DependencyScanning/DependencyScanningTool.h
@@ -205,7 +205,7 @@ class FullDependencyConsumer : public DependencyConsumer {
       std::vector<P1689ModuleInfo> Requires) override {
     ModuleName = Provided ? Provided->ModuleName : "";
     llvm::transform(Requires, std::back_inserter(NamedModuleDeps),
-                    [](const auto &Module) { return Module.ModuleName; });
+                    [](const auto &Module) -> const auto & { return Module.ModuleName; });
   }
 
   TranslationUnitDeps takeTranslationUnitDeps();

Copy link

github-actions bot commented Aug 15, 2025

✅ With the latest revision this PR passed the C/C++ code formatter.

Copy link
Contributor

@naveen-seth naveen-seth left a comment

Choose a reason for hiding this comment

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

LGTM

[](const auto &Module) { return Module.ModuleName; });
llvm::transform(
Requires, std::back_inserter(NamedModuleDeps),
[](const auto &Module) -> const auto & { return Module.ModuleName; });
Copy link
Collaborator

Choose a reason for hiding this comment

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

Ultimately, you're doing a copy either way: you're inserting a string into an std::vector<std::string>. At best this saves a move constructor call.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

clang Clang issues not falling into any other category

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants