Skip to content

Conversation

@JackStouffer
Copy link
Contributor

@JackStouffer JackStouffer commented Jan 4, 2018

See #5991 (comment)

While in this instance, the code compiled just fine, the problem is that a const ref parameter in formatObject ended up being copied in said function if FormatSpec wasn't marked as ref. This poses a problem because FormatSpec is quite a large struct, not to mention the fact that we've already gone through the trouble of making a pointer to one, so making a copy is just extra work.

@dlang-bot
Copy link
Contributor

Thanks for your pull request, @JackStouffer!

Bugzilla references

Your PR doesn't reference any Bugzilla issue.

If your PR contains non-trivial changes, please reference a Bugzilla issue or create a manual changelog.

@JackStouffer JackStouffer changed the title Changed all instances of FormatSpec to const ref Changed all instances of FormatSpec in toString to const ref Jan 4, 2018
@JackStouffer
Copy link
Contributor Author

This is technically a breaking change mind you. However, it's very rare that a user would call these methods directly and not just use to or format.

@JackStouffer JackStouffer removed the Review:Trivial typos, formatting, comments label Jan 4, 2018
@andralex
Copy link
Member

andralex commented Jan 4, 2018

The size of FormatSpec is 72 bytes making the argument difficult. Might be better to just compress it to 64 bytes...

I'll accept this anyway.

@dlang-bot dlang-bot merged commit c8b8a0a into dlang:master Jan 4, 2018
@JackStouffer JackStouffer deleted the toString-ref branch January 4, 2018 18:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants