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

Is NoRepType right for immutable vectors? #204

Closed
andy-morris opened this issue Apr 3, 2018 · 2 comments
Closed

Is NoRepType right for immutable vectors? #204

andy-morris opened this issue Apr 3, 2018 · 2 comments

Comments

@andy-morris
Copy link
Contributor

andy-morris commented Apr 3, 2018

The following GHCI session illustrates what I'm talking about:

λ» :set -XOverloadedLists 
λ» :m Data.Set Data.Vector Data.Generics.Uniplate.Data

λ» getInts xs = universeBi xs :: [Int]

λ» getInts ([1, 2, 3] :: [Int])
[1,2,3]

λ» getInts ([1, 2, 3] :: Set Int)
[1,2,3]

λ» getInts ([1, 2, 3] :: Vector Int)
[]

Since all vectors declare themselves to be NoRepTypes, the uniplate library (and probably others) immediately stop traversing on encountering a vector. This seems to me like it's probably not the intended behaviour for immutable vectors. (For mutable ones it's a different matter, of course.)

So, unless I'm mistaken, I'd like to suggest that immutable vector types follow the lead of Set and friends, and have Data instances along the lines of the following (but presumably defined once and for all in ...Generic like now):

instance Data a => Data (Vector a) where
    gfoldl f z m   = z fromList `f` toList m
    toConstr _     = fromListConstr -- was: error "toConstr"
    gunfold k z c  = case constrIndex c of -- was: error "gunfold"
        1 -> k (z fromList)
        _ -> error "gunfold"
    dataTypeOf _   = vectorDataType -- was: mkNoRepType "Data.Vector.Vector"
    dataCast1 f    = gcast1 f

fromListConstr :: Constr
fromListConstr = mkConstr vectorDataType "fromList" [] Prefix

vectorDataType :: DataType
vectorDataType = mkDataType "Data.Vector.Vector" [fromListConstr]
@RyanGlScott
Copy link
Member

RyanGlScott commented Apr 3, 2018

Indeed, the current Data instances seem strangely inconsistent, since we implement gfoldl f z m = z fromList f toList m but don't implement other methods in a similar fashion.

Your proposed implementation sounds sensible to me—care to make a pull request?

@andy-morris
Copy link
Contributor Author

You probably saw, but if not, I've put it in #205.

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

No branches or pull requests

2 participants