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

Make cast generated enums to i32 non-scalar #251

Merged
merged 1 commit into from
Apr 25, 2016

Conversation

EPashkin
Copy link
Member

No description provided.

@EPashkin
Copy link
Member Author

Its second part of enum as i32. See gtk-rs/examples#88

@EPashkin
Copy link
Member Author

AppVeyor build succeeded, @gkoz, please, merge this

@EPashkin
Copy link
Member Author

Seems I missread matches, need more (_)

@EPashkin
Copy link
Member Author

Or not?

@EPashkin
Copy link
Member Author

I tried micro-optimisation on match.

@gkoz
Copy link
Member

gkoz commented Apr 25, 2016

__Nonexhaustive(_) => panic!(),

This got me thinking: if we somehow get an unexpected variant from the library, should it be possible to pass it back unchanged? This would require complicated workarounds and making enums much fatter.

@EPashkin
Copy link
Member Author

EPashkin commented Apr 25, 2016

We can add flag for some enums, that convert it to __Nonexhaustive(i32) on from_glib and decode it in to_glib

@EPashkin
Copy link
Member Author

EPashkin commented Apr 25, 2016

These enums still defended against direct converting, so I don't see any problems.

@gkoz
Copy link
Member

gkoz commented Apr 25, 2016

Problem is we can't allow putting arbitrary i32s there and that's where complicated workarounds come in.

@EPashkin
Copy link
Member Author

Sorry, don't understand :(

@gkoz
Copy link
Member

gkoz commented Apr 25, 2016

Enum members can't have private fields.
Type safety (and likely memory safety too) require that a user can't just put any random value in that variant and also can't copy it from one enum into another, so it'd be along the lines of

pub struct Discriminant<T> {
    value: i32,
    _dummy: PhantomData<*const T>,
};

impl<T> struct Discriminant<T> {
    // there shouldn't be a safe way to put arbitrary discriminants into enums
    pub unsafe fn new(value: i32) -> Self {
        Discriminant {
            value: value,
            _dummy: PhantomData::new(),
        }
    }
}

pub enum Foo {
    Member1,
    Member2,
    Member3,
    __Nonexhaustive(Discriminant<Self>),
}

This looks like it's not worth the trouble at this point.

@EPashkin
Copy link
Member Author

IMHO it not much related to this PR ;)
Currently we safe from direct converting enum <-> int (not checked int -> enum but FromPrimitive trait seems discarded),
way make defence against enum1 -> int -> enum2 you show IMHO not needed until some example arises.

@gkoz gkoz merged commit a3f05e3 into gtk-rs:master Apr 25, 2016
@EPashkin EPashkin deleted the block_enum_to_i32 branch April 25, 2016 14:03
@EPashkin EPashkin mentioned this pull request Nov 5, 2016
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