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

[Relay] Add hd,tl,nth for list in Prelude #2771

Merged
merged 1 commit into from
Mar 14, 2019
Merged

Conversation

wweic
Copy link
Contributor

@wweic wweic commented Mar 11, 2019

Adding these common helpers for list. Following OCaml's name.

@wweic
Copy link
Contributor Author

wweic commented Mar 11, 2019

@MarisaKirisame @slyubomirsky Please take a look.

@MarisaKirisame
Copy link
Contributor

MarisaKirisame commented Mar 11, 2019

LGTM.
I think this is a good time to discuss partiality introduced by pattern matching. rn they fail with internal error, but it pass typechecking without warning or error.

I think we should sort of add a 'partial' annotation and require match to be annotated as such, if it is partial.

It could be implemented by having a boolean field in Match.

Another option is, provide an explicit panic construct that signal a runtime error, force all match to be total, provide a partial match helper that add a default case that throw "not matched".

@jroesch @tqchen wdyt?

@wweic
Copy link
Contributor Author

wweic commented Mar 11, 2019

@MarisaKirisame Or we can follow other general purpose language(like OCaml) to add exceptions, for cases we don't expect to match, throw an exception. I guess this is similar with the panic construct you suggested.

@MarisaKirisame
Copy link
Contributor

@wweic there's two orthgonal issue:
0: what to do at runtime when incomplete match come up
1: how do we provide safety mechanism to avoid user accidentally writing incomplete match

for ocaml, (0: is exception, and 1: is compiler warning).

I agree with your suggestion on (0:), but I am not sure about following ocaml on (1:).

@slyubomirsky
Copy link
Contributor

The code, other than the open question of how to handle incomplete matches, LGTM

For my part, I think if we do want to allow incomplete matches (I would prefer Relay to always be total, but that could be impractical), then we should check at compile-time that matches be complete and require the user to annotate deliberately partial matches.

@tqchen
Copy link
Member

tqchen commented Mar 14, 2019

Thanks @MarisaKirisame @slyubomirsky @wweic this is now merged

wweic added a commit to wweic/tvm that referenced this pull request Mar 20, 2019
wweic added a commit to neo-ai/tvm that referenced this pull request Mar 20, 2019
@wweic wweic deleted the wweic-add-nth branch April 1, 2019 06:06
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.

4 participants