Skip to content

Next crystal#36

Closed
will wants to merge 2 commits intomasterfrom
next-crystal
Closed

Next crystal#36
will wants to merge 2 commits intomasterfrom
next-crystal

Conversation

@will
Copy link
Copy Markdown
Owner

@will will commented Apr 14, 2016

I'm having trouble with some of the changes for the new stuff in crystal-lang/crystal#2443

I don't think @read_event is ever set in @ysbaddaden 's #31 so commenting it out in 1f52767 works

Other than that though, I've been stuck on a few cases with crystal spec and was wondering if @asterite or anyone could spare a few minutes.

The two things so far:

  • currently getting the types for @rows in result.cr
  • before that the fields[col].decoder.decode(val_ptr.to_slice(size)) line was complaining that a fields[col] was returning procs, but that isn't happening for me right now, also it doesn't return procs :/. That may have just been some other error I lost when I started over.

@asterite
Copy link
Copy Markdown
Contributor

For @rows it seems T is something like Tuple(Int32.class, String.class) and you need @rows to be Array(Tuple(Int32, String))... though I'm also seeing this:

     |  +- generic class PG::Result(Array(Array(JSON::Type) | Bool | Float32 | Float64 | Hash(String, JSON::Type) | Int32 | Int64 | String | Time | Nil)) (40 bytes)
     |         @types  : Array(Array(JSON::Type) | Bool | Float32 | Float64 | Hash(String, JSON::Type) | Int32 | Int64 | String | Time | Nil) (8 bytes)
     |         @res    : Pointer(Void)                                                                                                        (8 bytes)
     |         @fields : Array(PG::Result::Field)?                                                                                            (8 bytes)
     |         @rows   : Array(Array(Bool | Float32 | Float64 | Int16 | Int32 | Int64 | JSON::Any | Slice(UInt8) | String | Time | Nil))?     (8 bytes)

Here T is not a tuple and @rows is more or less T.

Right now there's no way to express that. But since rows will always be consumed, because I see very method in Result eventually calling rows, why not compute the rows and pass it to PG::Result, with T being the types of rows.

Then I think PG::Result::Row will maybe need to be generic too because you won't be able to just say @result : Result because it's generic.

The problem about the decoder is a bug, currently with this:

class Foo(T)
  def initialize
    @array = Array.new(10, 0)
  end

  def array
    @array
  end
end

the compiler infers @array to be Array, without a generic type argument, and that's not currently possible so the error is garbage. I'll fix this in a few minutes.

@asterite
Copy link
Copy Markdown
Contributor

I'm also trying to think of a way to pass from a class type to its instance type, but I can't find it with the current language.

We have T.class in the type grammar to pass from instance to class. Maybe we are missing T.instance to do the inverse. With that you'll be able to do @rows : Array(T.instance). Maybe a better name for that could be T.new, so you'd write @rows : Array(T.new). What do you think?

@will
Copy link
Copy Markdown
Owner Author

will commented Apr 14, 2016

I managed to get all the specs passing. The row problem was fixed by removing the memoization.

I also had to make Result::Row into Result::Row(T), but then it lost privileges to call the protected method

@asterite
Copy link
Copy Markdown
Contributor

Woops, that's a bug too. I'll fix it 😊

@ysbaddaden
Copy link
Copy Markdown
Contributor

Yes, please drop @read_event I didn't clean it up in my patch, sorry.

@will will force-pushed the next-crystal branch 2 times, most recently from cafe233 to cafe5e9 Compare April 17, 2016 18:11
@will will closed this May 5, 2016
@will will deleted the next-crystal branch November 5, 2017 03:16
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.

3 participants