Pass (large) strings by value and move if they are to be consumed

[chriseth]

Especially when setting the source input of the compiler, these strings are passed by const reference. This forces a copy which would actually be unnecessary. The recommended way of passing data that is consumed/stored by the function/method is to pass by value and move into the member variable.

There are several instances where solidity sources stored in strings are copied instead of moved like that, for example in CompilerStack::addSource. This should be fixed.

[maxrobot]

Hi, quick question for further clarification...

The function you mention it is defined as such:

CompilerStack::addSource(string const& _name, ...)

Variable _name is passed as a constant reference, which you say forces an unnecessary copy.

It is my understanding that in cpp passing by constant reference does not cause a copy.

Citing IBM:
Pass-by-references is more efficient than pass-by-value, because it does not copy the arguments.

So my question is where does the unnecessary copy occur?

[chriseth]

You are right that passing by const& does not cause a copy when calling the function, but if the function consumes the argument, it has to copy it somewhere, and this is where the copy occurs. If you pass by value instead, it can result in a copy and a move in the worst case, but it two moves in the good case, and this is what we aim for. The article you cite is probably outdated.

You might be able to find more information here: https://stackoverflow.com/questions/270408/is-it-better-in-c-to-pass-by-value-or-pass-by-constant-reference

[maxrobot]

Thank you very much!

[gitcoinbot]

Issue Status: 1. Open 2. Started 3. Submitted 4. Done

---

Work has been started.

These users each claimed they can complete the work by 4 days, 19 hours ago.
Please review their action plans below:

1) g-r-a-n-t has been approved to start work.

Looks pretty straight forward.

AP:

• modify any functions taking in the source such that it passes by value
• briefly check if this is happening anywhere else

Learn more on the Gitcoin Issue Details page.

[gitcoinbot]

@g-r-a-n-t Hello from Gitcoin Core - are you still working on this issue? Please submit a WIP PR or comment back within the next 3 days or you will be removed from this ticket and it will be returned to an ‘Open’ status. Please let us know if you have questions!

• reminder (3 days)
• escalation to mods (6 days)

Funders only: Snooze warnings for 1 day | 3 days | 5 days | 10 days | 100 days

[gitcoinbot]

@g-r-a-n-t Hello from Gitcoin Core - are you still working on this issue? Please submit a WIP PR or comment back within the next 3 days or you will be removed from this ticket and it will be returned to an ‘Open’ status. Please let us know if you have questions!

• reminder (3 days)
• escalation to mods (6 days)

Funders only: Snooze warnings for 1 day | 3 days | 5 days | 10 days | 100 days

[g-r-a-n-t]

Hello @gitcoinbot, this is currently under review.

[gitcoinbot]

Issue Status: 1. Open 2. Started 3. Submitted 4. Done

---

Work for 200.0 DAI (200.0 USD @ $1.0/DAI) has been submitted by:

1. @g-r-a-n-t

@vs77bb please take a look at the submitted work:

• PR by @g-r-a-n-t

---

• Learn more on the Gitcoin Issue Details page
• Want to chip in? Add your own contribution here.
• Questions? Checkout Gitcoin Help or the Gitcoin Slack
• $100,296.79 more funded OSS Work available on the Gitcoin Issue Explorer

[g-r-a-n-t]

@vs77bb Could you please take a look at this work?

[gitcoinbot]

⚡️ A *Gitcoin Tree* Kudos has been sent to @g-r-a-n-t for this issue from @vs77bb. ⚡️ The sender had the following public comments: Thanks for your patience, and great work! Nice work @g-r-a-n-t! Your Kudos has automatically been sent in the ETH address we have on file.

[gitcoinbot]

Issue Status: 1. Open 2. Started 3. Submitted 4. Done

---

The funding of 200.0 DAI (200.0 USD @ $1.0/DAI) attached to this issue has been approved & issued to @g-r-a-n-t.

• Learn more on the Gitcoin Issue Details page
• Questions? Checkout Gitcoin Help or the Gitcoin Slack
• $97,692.44 more funded OSS Work available on the Gitcoin Issue Explorer

[chriseth]

Solved by #6490