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

Don't implement DerefMut on TypedValue #255

Merged
merged 4 commits into from Nov 15, 2017

Conversation

Projects
None yet
3 participants
@sdroege
Member

sdroege commented Nov 15, 2017

It allows replacing the underlying Value inside the TypedValue with one
of a different type and breaks type-safety:

  let mut meh: TypedValue<i32> = (&123i32).into();
  let meh2: Value = (&1u8).to_value();
  *meh = meh2;

Fixes https://github.com/gtk-rs/glib/issues/254

Also includes some trivial cleanup.

sdroege added some commits Nov 15, 2017

Don't implement DerefMut on TypedValue
It allows replacing the underlying Value inside the TypedValue with one
of a different type and breaks type-safety:

  let mut meh: TypedValue<i32> = (&123i32).into();
  let meh2: Value = (&1u8).to_value();
  *meh = meh2;

Fixes #254
Implement fmt::Debug for Value/TypedValue by using Formatter::debug_t…
…uple()

This takes into account all formatting parameters as needed.

@sdroege sdroege changed the title from Don't implement DerefMut on TypedValu to Don't implement DerefMut on TypedValue Nov 15, 2017

Make TypedValue<T> #[repr(C)]
This allows to use them directly wherever a GValue is used in FFI by
transmuting
@GuillaumeGomez

This comment has been minimized.

Show comment
Hide comment
@GuillaumeGomez
Member

GuillaumeGomez commented Nov 15, 2017

👍

@EPashkin

This comment has been minimized.

Show comment
Hide comment
@EPashkin

EPashkin Nov 15, 2017

Member

@sdroege Thanks,
👍 too

Member

EPashkin commented Nov 15, 2017

@sdroege Thanks,
👍 too

@GuillaumeGomez GuillaumeGomez merged commit 83f162e into gtk-rs:master Nov 15, 2017

2 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@GuillaumeGomez

This comment has been minimized.

Show comment
Hide comment
@GuillaumeGomez

GuillaumeGomez Nov 15, 2017

Member

Thanks!

Member

GuillaumeGomez commented Nov 15, 2017

Thanks!

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