Skip to content

Fix ownership issue with KeyOrValue#786

Merged
cmyr merged 1 commit intomasterfrom
fixup-key-or-value
Apr 1, 2020
Merged

Fix ownership issue with KeyOrValue#786
cmyr merged 1 commit intomasterfrom
fixup-key-or-value

Conversation

@cmyr
Copy link
Member

@cmyr cmyr commented Mar 31, 2020

A Key<T> does not always return a value, T; it can return a borrowed
value. This is only really exercised by String, which we return by reference.

In a situation like this, we cannot have From<T> for KeyOrValue<T> as well as From<Key<T>> for KeyOrValue<T>, because the owned type may be different then the type used by the key.

This is fixed by explicitly implementing From in terms of our ValueType trait, so that we can reference the owned type directly.

This should unblock #785.

cc @futurepaul, who might be curious about this.

A Key<T> does not always return a value, T; it can return a borrowed
value. This is only really exercised by String, which we return by reference.

In a situation like this, we cannot have From<T> for KeyOrValue<T> as well as From<Key<T>> for KeyOrValue<T>, because the owned type may be different then the type used by the key.

This is fixed by explicitly implementing From in terms of or ValueType trait, so that we can reference the owned type directly.
@cmyr cmyr force-pushed the fixup-key-or-value branch from 4b85e3e to b3ab0af Compare March 31, 2020 15:32
Copy link
Collaborator

@futurepaul futurepaul left a comment

Choose a reason for hiding this comment

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

Tests pass, styled_text runs, and the associated type means the rust compiler can guard against mis-use. Thanks for the zulip walkthrough!

@cmyr cmyr merged commit 83247eb into master Apr 1, 2020
@cmyr cmyr deleted the fixup-key-or-value branch April 1, 2020 13:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants