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

documentation for warning 52 #7245

Closed
vicuna opened this issue Apr 27, 2016 · 2 comments

Comments

Projects
None yet
2 participants
@vicuna
Copy link

commented Apr 27, 2016

Original bug ID: 7245
Reporter: william
Assigned to: @gasche
Status: resolved (set by @mshinwell on 2016-09-08T09:19:12Z)
Resolution: fixed
Priority: low
Severity: minor
Target version: 4.04.0 +dev / +beta1 / +beta2
Category: documentation
Monitored by: @hcarty

Bug description

It would be nice to replace "52 Fragile constant pattern" by "52 Fragile constant pattern (see detailed section below)"

Also, I would replace the following explanations :

"If your code raises this warning, you should not change the way you test for the specific string to avoid the warning (for example using a string equality inside the right-hand-side instead of a literal pattern), as your code would remain fragile. You should instead enlarge the scope of the pattern by matching on all possible values. This may require some care: if the scrutinee may return several different cases of the same pattern, or raise distinct instances of the same exception, you may need to modify your code to separate those several cases."

By this :
If your code raises this warning, you should not change the way you test for the specific string to avoid the warning (for example using a string equality inside the right-hand-side instead of a literal pattern), as your code would remain fragile. You should instead enlarge the scope of the pattern by matching on all possible values, such as in this example :

let warning = function
| Foo _ -> 0
| _ -> 1

This may require some care: if the scrutinee may return several different cases of the same pattern, or raise distinct instances of the same exception, you may need to modify your code to separate those several cases. For example [...]

@vicuna

This comment has been minimized.

Copy link
Author

commented Apr 28, 2016

Comment author: @gasche

I implemented the proposed changes in
#565

@vicuna

This comment has been minimized.

Copy link
Author

commented Sep 8, 2016

Comment author: @mshinwell

#565 has been merged

@vicuna vicuna closed this Sep 8, 2016

@vicuna vicuna added this to the 4.04.0 milestone Mar 14, 2019

@vicuna vicuna added the bug label Mar 20, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.