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

@enum implementation #5842

Closed
wants to merge 1 commit into from
Closed

@enum implementation #5842

wants to merge 1 commit into from

Conversation

filmackay
Copy link

Here's an attempt to improve the @enum implementation (still in examples for now). You can do things like:

julia> include ("[..]/julia/examples/enum.jl")

julia> @enum Sort [:sortnone=>0, :sortasc=>1, :sortdesc=>2]

julia> convert(Cint, sortasc)
1

julia> names(Sort)
3-element Array{Symbol,1}:
 sortdesc
 sortasc
 sortnone

julia> sortnone
Sort(0)

julia> sortnone < sortasc
true

julia> sortnone == sortdesc
false

julia> convert(Sort, 1)
Sort(1)

julia> convert(Cint, sortdesc)
2

Issues:

  • I seem to need to keep the symbol in the enum - so I can show a better name for each value
  • I originally had a type parameter to define the Cint equivalent used. Due to the complexity I decided to start with simple Cint first.

Feedback welcome :)

@filmackay
Copy link
Author

Just realised the second convert() can be pulled out and made generic. Apologies for leaving the Sort example in the code too - will need to fix that :)

@vtjnash
Copy link
Member

vtjnash commented Feb 18, 2014

see also #2988 for a more comprehensive implementation of enum functionality. adding this dict-like mapping would be good there too

@filmackay
Copy link
Author

Thanks @vtjnash - I did look at #2988 but opted to just attack the pure enum, and leave enum flags for another PR. I also wanted to avoid a few other things like making the enum type iterable etc.

The thing I don't like about my PR is the fact that enum members are not scoped. As discussed on julia-users this is waiting for getfield overloads. https://groups.google.com/forum/#!topic/julia-users/mk4xWQf4I8U

@StefanKarpinski
Copy link
Member

Recording my proposal here so people don't have to click through and find it. With overloadable field access, we can just create a Direction type and define (automatically) the following methods:

getfield(::Type{Direction}, ::Field{:NORTH}) = Direction(0)
getfield(::Type{Direction}, ::Field{:EAST})  = Direction(1)
getfield(::Type{Direction}, ::Field{:WEST})  = Direction(2)
getfield(::Type{Direction}, ::Field{:SOUTH}) = Direction(3)

That would make Direction simultaneously a type and let it act like a "namespace" in the sense that you can do Direction.NORTH and get the value Direction(0). The definition of the type would be something like this:

immutable Direction <: Enum
  value::Cint
end

Enum values are thus completely memory compatible with C enums. Not sure if we need a new keyword for this – especially considering that we'll want something very similar for flags – i.e. enums where the values are powers of two. Having @enum and @flag macros may well be enough. But this definitely needs the ability to overload field access to make it work completely naturally.

@tknopp
Copy link
Contributor

tknopp commented Feb 20, 2014

I like Stefans proposal a lot and hope that it finds consensus. What remains kind of open is the question how the interface (i.e. the @enum and @flag macros) look like. It would be great if @enum would by default assume an increasing integer range starting from zero but it would also be cool if one could define arbitrary values. For @flag one might think of specifying which bit is toggled (between 1 and 32) but this seems not to be that important.

@filmackay
Copy link
Author

Is this this getfield change a significant/difficult one @StefanKarpinski? Is this #5848?

@StefanKarpinski
Copy link
Member

That is an initial take on it.

@JeffBezanson
Copy link
Member

I am suddenly full of dread. If one were to define

getfield(::Type{Direction}, ::Field{:types}) = ...

things would be incredibly broken. Or we'd have to be careful to use Core.getfield for any kind of reflection.

@toivoh
Copy link
Contributor

toivoh commented Feb 21, 2014

It seems like either that (which might be sane, though perhaps not terribly beautiful), or not allow to override getfield for existing fields.

But shouldn't we really abstract things like .types and .names anyway, and provide getter functions for them instead? If we do that consistently, it doesn't matter that the getters call Core.getfield. A bonus would be that typesof (or what we would call it) would only be defined for types. Though I guess it's a pretty big change...

@StefanKarpinski
Copy link
Member

I am suddenly full of dread.

I was also a bit surprised – and a bit worried – by how powerful the field overloading feature is.

@StefanKarpinski
Copy link
Member

I know you don't like underscore-oriented programming, but we could consider field names with some number/pattern of underscores to be reserved for the system.

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.

6 participants