Prevent warning 59 from triggering on Lazy of constants #714
Conversation
|
This looks correct to me, but I find the double-checking of the argument approximation for the common cases a bit ugly (checked once inside |
|
Yes that saddened me too. But maybe returning type compatibility =
| Incompatible (* Cannot be that case *)
| Compatible (* Can be that case but can be other cases *)
| Selected (* Can be that case and cannot be any other case *)can help lighten the code. I'll give it a try. |
| (* In theory symbol cannot contain integers but this shouldn't | ||
| matter as this will always be an imported approximation *) | ||
| true | ||
|
|
There was a problem hiding this comment.
No blank lines in functions please!
There was a problem hiding this comment.
It we are discussing style, I'd suggest to drop the custom .ocp-indent in middle-end/ and adopt the global configuration. I find the lack of extra indentation for pattern matching "actions" (w.r.t. the pattern) makes the code much harder to read. I would also suggest to drop useless parentheses around or-patterns.
|
@chambart Maybe try to make the constructor names sufficiently descriptive that you don't need the comments in the type declaration? |
|
@lpw25, @mshinwell Is that better ? Do you approve it now ? |
Lazy.force (lazy (fun x -> x)) was triggering this warning because the code generated by Lazy.force modifies its argument in a dead branch. This allows to recognize this branch as dead, and eliminate it.
chambart
force-pushed
the
lazy_warning_59
branch
from
11fbd39
to
6ee86d5
Compare
|
Rebased |
|
This looks correct to me. Merging. |
Prevent warning 59 from triggering on Lazy of constants
Lazy.force (lazy (fun x -> x))was triggering this warning because thecode generated by Lazy.force modifies its argument in a dead branch.
This allows to recognize this branch as dead, and eliminate it.