Skip to content

Conversation

@abhishek-kaushik22
Copy link
Contributor

No description provided.

@llvmbot
Copy link
Member

llvmbot commented Nov 19, 2024

@llvm/pr-subscribers-coroutines

@llvm/pr-subscribers-llvm-transforms

Author: None (abhishek-kaushik22)

Changes

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

1 Files Affected:

  • (modified) llvm/include/llvm/Transforms/Coroutines/ABI.h (+4-4)
diff --git a/llvm/include/llvm/Transforms/Coroutines/ABI.h b/llvm/include/llvm/Transforms/Coroutines/ABI.h
index 8b83c5308056eb..0b2d405f3caec4 100644
--- a/llvm/include/llvm/Transforms/Coroutines/ABI.h
+++ b/llvm/include/llvm/Transforms/Coroutines/ABI.h
@@ -41,7 +41,7 @@ class BaseABI {
 public:
   BaseABI(Function &F, coro::Shape &S,
           std::function<bool(Instruction &)> IsMaterializable)
-      : F(F), Shape(S), IsMaterializable(IsMaterializable) {}
+      : F(F), Shape(S), IsMaterializable(std::move(IsMaterializable)) {}
   virtual ~BaseABI() = default;
 
   // Initialize the coroutine ABI
@@ -67,7 +67,7 @@ class SwitchABI : public BaseABI {
 public:
   SwitchABI(Function &F, coro::Shape &S,
             std::function<bool(Instruction &)> IsMaterializable)
-      : BaseABI(F, S, IsMaterializable) {}
+      : BaseABI(F, S, std::move(IsMaterializable)) {}
 
   void init() override;
 
@@ -80,7 +80,7 @@ class AsyncABI : public BaseABI {
 public:
   AsyncABI(Function &F, coro::Shape &S,
            std::function<bool(Instruction &)> IsMaterializable)
-      : BaseABI(F, S, IsMaterializable) {}
+      : BaseABI(F, S, std::move(IsMaterializable)) {}
 
   void init() override;
 
@@ -93,7 +93,7 @@ class AnyRetconABI : public BaseABI {
 public:
   AnyRetconABI(Function &F, coro::Shape &S,
                std::function<bool(Instruction &)> IsMaterializable)
-      : BaseABI(F, S, IsMaterializable) {}
+      : BaseABI(F, S, std::move(IsMaterializable)) {}
 
   void init() override;
 

@abhishek-kaushik22
Copy link
Contributor Author

@ChuanqiXu9 @TylerNowicki can you please review?

Copy link
Member

@Flakebi Flakebi left a comment

Choose a reason for hiding this comment

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

Looks good to me

Copy link
Contributor

@Dinistro Dinistro left a comment

Choose a reason for hiding this comment

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

Drive by: Make sure to update the commit message before you merge such that it follows the guidelines.

@abhishek-kaushik22 abhishek-kaushik22 changed the title Use std::move to avoid copying [Coroutines] Use std::move to avoid copying Nov 19, 2024
Copy link
Member

@ChuanqiXu9 ChuanqiXu9 left a comment

Choose a reason for hiding this comment

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

Please add NFC to the title.

@abhishek-kaushik22 abhishek-kaushik22 changed the title [Coroutines] Use std::move to avoid copying [NFC] [Coroutines] Use std::move to avoid copying Nov 20, 2024
@abhishek-kaushik22
Copy link
Contributor Author

@Flakebi @ChuanqiXu9 can you please merge?

@ChuanqiXu9 ChuanqiXu9 merged commit c6bce68 into llvm:main Nov 20, 2024
9 of 11 checks passed
@ChuanqiXu9
Copy link
Member

@Flakebi @ChuanqiXu9 can you please merge?

Done

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

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants