Turn on a bunch of cippy ints #127
Conversation
30f125f Satisfy program with optional pruning (Michael Zaikin) Pull request description: This PR exposes an API for satisfying a program with pruning given an optional environment. The naming might be a bit misleading (maybe `satisfy_pruned` would be better?), pls let me know if you want to change.
We will set these all to "warn" in the following commits.
These can be immediately turned on without code changes.
This is really just a stylistic thing, but it makes the code shorter so I think we should use it.
When casting between numeric types, if the conversion is infallible then we can use `from`. This means that if the types change to make the conversion lossy, we'll get a compiler error rather than the logic silently becoming wrong.
Also a style thing, but makes the code more idiomatic.
BasePattern::as_identifier returns an Option; when iterating we can filter out the Nones with `filter_map`. We were instead using `flat_map`, which is much more general: it turns the Options into iterators (which will yield 0 or 1 items) and then yields everything from the iterators. This is potentially slower and more complicated to read.
This is a stylistic lint which you could argue either way, but I like having it on for consistency.
|
cc @canndrew can you take a look at this one? |
|
And @uncomputable since this is kinda your crate and I'm making style changes. |
| @@ -1 +1,3 @@ | |||
| msrv = "1.78.0" | |||
| # We have an error type, `RichError`, of size 144. This is pushing it but probably fine. | |||
| large-error-threshold = 145 | |||
There was a problem hiding this comment.
Just boxing the Error in RichError would cut down the size a fair bit. I was gonna do a PR to replace the String errors with anyhow::Error in a few places, so I'll include it in that.
There was a problem hiding this comment.
That sounds good. If you can remove this clippy.toml line I'd appreciate it!
Also, because this is both a library and a CLI tool, it may be a judgement call where to use anyhow ... I'd prefer we not use it on any "library errors" and only on "application errors".
Historically we've manually done all our errors, which is a bit tedious. I wouldn't mind if you wanted to introduce thiserror here. In rust-simplicity probably not, because we're much more agressive about pruning dependencies there.
There was a problem hiding this comment.
Yeah anyhow is kind of a lazy hack. Just less so than using String. I agree that libraries should use proper errors that you can actually match on. I'll fix up the PR I just opened.
This is the first part of a project to turn on a ton of pedantic Clippy lints. We can choose not to do some if they're too annoying. There are about 20 left after this.