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

change block to Proc #25

Merged
merged 1 commit into from May 5, 2019
Merged

change block to Proc #25

merged 1 commit into from May 5, 2019

Conversation

moba1
Copy link
Collaborator

@moba1 moba1 commented May 4, 2019

Contents of change

Changed following methods which received &block to receive Proc.

  • value_or
  • fmap
  • bind
  • map_or

Reason

In the current code, following code can not run correctly:

Left.new(1).fmap {|x| x + 1}

The cause lies with Crystal compiler can't infer function type(compiler can't infer T of &block : T -> U).
So instead of receiving a &block, change it to receive a Proc.
As a result, value_or(&block : ->U) was deleted because it became impossible to overload.

@moba1 moba1 requested a review from alex-lairan May 4, 2019 13:50
Copy link
Owner

@alex-lairan alex-lairan left a comment

Choose a reason for hiding this comment

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

In Crystal a Block is like a Proc, but with a simpler syntax.

Why not allow both ?

Edit : sorry, I've read the reason.
Maybe there is a way to have both worlds. I will try myself today.

@alex-lairan
Copy link
Owner

I tried and got this :

icr(0.26.1) > require "./src/monads"
 => ok
icr(0.26.1) > left = Monads::Left.new(5)
 => Monads::Left(Int32){5}
icr(0.26.1) > left.fmap { |x| x * 2 }
 => Monads::Left(Int32){5}
icr(0.26.1) > left.fmap(->(x : Int32) { x * 2 })
 => Monads::Left(Int32){5}
icr(0.26.1) > right = Monads::Right.new(5)
 => Monads::Right(Int32){5}
icr(0.26.1) > right.fmap { |x| x * 2 }
 => Monads::Right(Int32){10}
icr(0.26.1) > right.fmap(->(x : Int32) { x * 2 })
 => Monads::Right(Int32){10}

And for Crystal 0.28

icr(0.28.0) > require "./src/monads"
 => ok
icr(0.28.0) > left = Monads::Left.new(5)
 => Monads::Left(Int32){5}
icr(0.28.0) > left.fmap { |x| x * 2 }
 => Monads::Left(Int32){5}
icr(0.28.0) > left.fmap(->(x : Int32) { x * 2 })
 => Monads::Left(Int32){5}
icr(0.28.0) > right = Monads::Right.new(5)
 => Monads::Right(Int32){5}
icr(0.28.0) > right.fmap { |x| x * 2 }
 => Monads::Right(Int32){10}
icr(0.28.0) > right.fmap(->(x : Int32) { x * 2 })
 => Monads::Right(Int32){10}

Can you give me your errors ?

@moba1
Copy link
Collaborator Author

moba1 commented May 5, 2019

I implement following code :

class Left(E) < Either(E, Nil)
  def fmap(&block : T -> U) forall T, U
    self
  end
end

This code was implemented for the purpose of enabling execution of the same block with Right(T) and Left(E):

# a is Left(String) or Right(Int32)
a.fmap {|x| x + 1}

But, if a is Left (String), Crystal can't run this correctly under the current code.
Also, in the above implementation, Crystal can't infer what type comes to fmap first argument, and Crystal compiler has bug in type inference: T == Nil.
As a result, Crystal can't infer type T correctly even with the above implementation, and return error: undifned method '+' for Nil.

@moba1
Copy link
Collaborator Author

moba1 commented May 5, 2019

I asked Forum whether Crystal can infer T from &block : T-> U under the code that returns self like the above.
Then I got answer that T should not be able to infer in the above code.

@alex-lairan
Copy link
Owner

Got it !

I opened an issue : #26

@alex-lairan alex-lairan merged commit 1a10ca5 into master May 5, 2019
@moba1 moba1 deleted the fix/change-block-to-proc branch May 5, 2019 15:11
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.

None yet

2 participants