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

Allow rescuing exceptions that include a module #14553

Merged
merged 7 commits into from
May 10, 2024

Conversation

Blacksmoke16
Copy link
Member

Resolves #14552

@straight-shoota
Copy link
Member

issue: Actually, I think the one-line change doesn't cut it. I hadn't though about union types, so the additional spec is very helpful to realize that. We need to make sure to accept a union of module types as well.

@Blacksmoke16
Copy link
Member Author

Blacksmoke16 commented May 2, 2024

👍 I'll add a spec for that, it definitely worked fine when I was manually playing around with things. I specifically wanted to ensure when you rescue via a module, the compiler knows the exception is an instance of that module in regards to the abstract methods on said module. Kinda makes me wish we could had intersection types to do rescue ex : Foo & Bar to rescue exceptions that include both vs one or the other 😅.

Copy link
Contributor

@ysbaddaden ysbaddaden left a comment

Choose a reason for hiding this comment

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

Maybe just one spec to verify that it won't match any module?

module Foo; end
module Bar; end

class Ex < Exception
  include Foo
end

x = 0
begin
  begin
    raise Ex.new("oh no")
  rescue Bar
    x = 1
  end
rescue ex
  ex.message
end 

@straight-shoota straight-shoota added this to the 1.13.0 milestone May 8, 2024
@straight-shoota straight-shoota merged commit d83e42f into crystal-lang:master May 10, 2024
61 checks passed
@Blacksmoke16 Blacksmoke16 deleted the rescue-modules branch May 31, 2024 01:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Allow rescuing exceptions based on included modules
3 participants