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

Do not display warning 40 in a non-ambiguous match #6731

Closed
vicuna opened this issue Dec 22, 2014 · 8 comments
Closed

Do not display warning 40 in a non-ambiguous match #6731

vicuna opened this issue Dec 22, 2014 · 8 comments

Comments

@vicuna
Copy link

vicuna commented Dec 22, 2014

Original bug ID: 6731
Reporter: @whitequark
Status: acknowledged (set by @damiendoligez on 2014-12-23T17:01:55Z)
Resolution: open
Priority: normal
Severity: feature
Version: 4.02.1
Category: middle end (typedtree to clambda)
Monitored by: @gasche

Bug description

The following match contains no ambiguity:

let lwt_log_level_to_enum =
  function
  | Lwt_log.Debug -> 0
  | Info -> 1
  | Notice -> 2
  | Warning -> 3
  | Error -> 4
  | Fatal -> 5

However, warning 40 is still displayed.

@vicuna
Copy link
Author

vicuna commented Dec 22, 2014

Comment author: @lpw25

I suspect this gets quite tricky for nested patterns, which is why we give no special treatment to sibling patterns (e.g. there was never any equivalent to the { Foo.bar; baz } record syntax). For similar reasons patterns like this will give a principality warning if the patterns require disambiguation.

@vicuna
Copy link
Author

vicuna commented Dec 23, 2014

Comment author: @gasche

I suspect this gets quite tricky for nested patterns

Barring GADTs, is there a fundamental reason why that would be trickier than nested record constructions?

@vicuna
Copy link
Author

vicuna commented Dec 23, 2014

Comment author: @lpw25

Barring GADTs, is there a fundamental reason why that would be trickier than nested record constructions?

I'm not sure what you mean by "nested record constructors".

@vicuna
Copy link
Author

vicuna commented Dec 23, 2014

Comment author: @gasche

It seems that { x = { a; b }; y; } displays the same sort of (non-)difficulties as the clauses (X A -> ... | X B -> ... | Y -> ... ).

@vicuna
Copy link
Author

vicuna commented Dec 23, 2014

Comment author: @lpw25

It seems that { x = { a; b }; y; } displays the same sort of (non-)difficulties as the clauses (X A -> ... | X B -> ... | Y -> ... ).

Well {a; b} is a single expression/pattern, so a and b can be reliably linked together syntactically. The trick which turns { Foo.a; b } into {Foo.a; Foo.b} can be done completely syntactically.

Whereas A and B in your second example are two separate patterns that are in some sense only related by their types. You can try to link them together via the idea that two patterns with the same type and the same "head" (X in your example) must have "sub-patterns" (A and B in your example) with the same type, but that clearly fails in the presence of things like existentials.

@github-actions
Copy link

This issue has been open one year with no activity. Consequently, it is being marked with the "stale" label. What this means is that the issue will be automatically closed in 30 days unless more comments are added or the "stale" label is removed. Comments that provide new information on the issue are especially welcome: is it still reproducible? did it appear in other contexts? how critical is it? etc.

@github-actions github-actions bot added the Stale label May 11, 2020
@trefis
Copy link
Contributor

trefis commented May 11, 2020

I don't think this is going to change in the near future (nor do I think that it should).
So I'm closing.

@trefis trefis closed this as completed May 11, 2020
@whitequark
Copy link
Member

whitequark commented May 11, 2020

Yes, on reflection I agree that this would not be a desirable feature due to the complex rules that would be necessary for disambiguation.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants