Skip to content
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

GString: Avoid copy for From<GString> for String impl #423

Merged
merged 1 commit into from Jan 12, 2019

Conversation

Projects
None yet
4 participants
@philn
Copy link
Contributor

philn commented Jan 12, 2019

Fixes #422

Show resolved Hide resolved src/gstring.rs Outdated
@EPashkin

This comment has been minimized.

Copy link
Member

EPashkin commented Jan 12, 2019

@philn Thansk, LGFM
Only possible change it do early return in nocopy case and use one call String::from(s.as_str())
but IMHO rust can optimize it yourself

@philn philn force-pushed the philn:master branch from 2be83ab to 8fb4567 Jan 12, 2019

@philn

This comment has been minimized.

Copy link
Contributor Author

philn commented Jan 12, 2019

Thanks, I updated the PR, code looks simpler now indeed!

Show resolved Hide resolved src/gstring.rs Outdated

@philn philn force-pushed the philn:master branch from 8fb4567 to bf30683 Jan 12, 2019

Show resolved Hide resolved src/gstring.rs Outdated
GString: Avoid copies when converting from/to String
Store the CString in an Option and seemlessly transfer it into a String when a
conversion is needed.

Also avoid to_vec() in the From<String> implementation because it performs a
copy.

Fixes #422

@philn philn force-pushed the philn:master branch from bf30683 to 205f437 Jan 12, 2019

@philn

This comment has been minimized.

Copy link
Contributor Author

philn commented Jan 12, 2019

CI is green and comments addressed :)

@EPashkin

This comment has been minimized.

Copy link
Member

EPashkin commented Jan 12, 2019

@philn Thanks, 👍

@GuillaumeGomez

This comment has been minimized.

Copy link
Member

GuillaumeGomez commented Jan 12, 2019

Thanks!

@GuillaumeGomez GuillaumeGomez merged commit ca3a766 into gtk-rs:master Jan 12, 2019

2 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.