Skip to content

[Clang][NFC] Use std::move to avoid copy #138073

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

Merged

Conversation

shafik
Copy link
Collaborator

@shafik shafik commented May 1, 2025

Static analysis flagged this code for using copy when we could use std::move.

Worth noting that CD.Message is a StringRef but Conflict.Message is std::string. Otherwise I would have used a temporary in place and avoid a local variable.

Static analysis flagged this code for using copy when we could use std::move.

Worth noting that CD.Message is a StringRef but Conflict.Message is std::string.
Otherwise I would have used a temporary in place and avoid a local variable.
@llvmbot llvmbot added clang Clang issues not falling into any other category clang:frontend Language frontend issues, e.g. anything involving "Sema" clang:modules C++20 modules and Clang Header Modules labels May 1, 2025
@llvmbot
Copy link
Member

llvmbot commented May 1, 2025

@llvm/pr-subscribers-clang-modules

@llvm/pr-subscribers-clang

Author: Shafik Yaghmour (shafik)

Changes

Static analysis flagged this code for using copy when we could use std::move.

Worth noting that CD.Message is a StringRef but Conflict.Message is std::string. Otherwise I would have used a temporary in place and avoid a local variable.


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

1 Files Affected:

  • (modified) clang/lib/Lex/ModuleMap.cpp (+2-1)
diff --git a/clang/lib/Lex/ModuleMap.cpp b/clang/lib/Lex/ModuleMap.cpp
index a1394fd3900b0..0b47ff5fa71f8 100644
--- a/clang/lib/Lex/ModuleMap.cpp
+++ b/clang/lib/Lex/ModuleMap.cpp
@@ -2010,7 +2010,8 @@ void ModuleMapLoader::handleConflict(const modulemap::ConflictDecl &CD) {
   Conflict.Id = CD.Id;
   Conflict.Message = CD.Message;
 
-  ActiveModule->UnresolvedConflicts.push_back(Conflict);
+  // FIXME: when we move to C++20 we should consider using emplace_back
+  ActiveModule->UnresolvedConflicts.push_back(std::move(Conflict));
 }
 
 void ModuleMapLoader::handleInferredModuleDecl(

@shafik shafik merged commit d7f98a4 into llvm:main May 1, 2025
15 checks passed
@shafik shafik deleted the move_to_avoid_copy_ModuleMapLoader_handleCoflict branch May 2, 2025 17:31
IanWood1 pushed a commit to IanWood1/llvm-project that referenced this pull request May 6, 2025
Static analysis flagged this code for using copy when we could use
std::move.

Worth noting that CD.Message is a StringRef but Conflict.Message is
std::string. Otherwise I would have used a temporary in place and avoid
a local variable.
IanWood1 pushed a commit to IanWood1/llvm-project that referenced this pull request May 6, 2025
Static analysis flagged this code for using copy when we could use
std::move.

Worth noting that CD.Message is a StringRef but Conflict.Message is
std::string. Otherwise I would have used a temporary in place and avoid
a local variable.
IanWood1 pushed a commit to IanWood1/llvm-project that referenced this pull request May 6, 2025
Static analysis flagged this code for using copy when we could use
std::move.

Worth noting that CD.Message is a StringRef but Conflict.Message is
std::string. Otherwise I would have used a temporary in place and avoid
a local variable.
GeorgeARM pushed a commit to GeorgeARM/llvm-project that referenced this pull request May 7, 2025
Static analysis flagged this code for using copy when we could use
std::move.

Worth noting that CD.Message is a StringRef but Conflict.Message is
std::string. Otherwise I would have used a temporary in place and avoid
a local variable.
Ankur-0429 pushed a commit to Ankur-0429/llvm-project that referenced this pull request May 9, 2025
Static analysis flagged this code for using copy when we could use
std::move.

Worth noting that CD.Message is a StringRef but Conflict.Message is
std::string. Otherwise I would have used a temporary in place and avoid
a local variable.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clang:frontend Language frontend issues, e.g. anything involving "Sema" 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.

3 participants