Add script for finding missing IO specializations
#683
Conversation
jeltsch
requested review from
dcoutts,
jorisdral,
mheinzel,
recursion-ninja and
wenkokke
as code owners
|
As a preliminary note, it would be good to change the script to require no parameters (ie inline the source glob) and perhaps rename it to start with lint, for consistency. |
|
I‘d like to keep the flexibility of being able to pass arbitrarily chosen Haskell source code to the script but realize that it would be good to have a script that just checks the present source code (probably of the main library only). What about keeping the script as it is and adding a wrapper script that passes the main library’s modules and have its name start with |
Sounds great! Another option would be to default to all Haskell files (or just |
| LC_COLLATE=C LC_CTYPE=C sed -En ' | ||
| :start | ||
| /^module / { | ||
| s/module +([^ ]*).*/\1 / |
There was a problem hiding this comment.
Might be good to explicitly match |$ after the capture group for clarity, to not depend on greediness?
There was a problem hiding this comment.
I used to put such extra information about delimiters into regular expressions, but now think that it’s fine to rely on greediness. After all, we’re relying on it in other situations already, where it’s hard or even impossible not to do so.
|
I think it might be a good idea to add dinner comments to the sed script that explain the logic a bit, not per se on a command by command basis, but to explain the algorithm. |
|
OTOH, it's sufficiently clear the script isn't malicious, so once there's a CI job that runs it on macOS and Windows, I'm happy to merge it under the understanding that it's not a complete guarantee on the absence of missing specialisations... which is true anyway, given the assumptions it makes. |
|
Are there cases where the script returns false positives, and if not now, is it conceivable that it could do so in the future? If so, it might be good to think about how such false positives could be addressed, specially if it's a CI job we run that can hold up PRs |
@jeltsch already floated the idea of an allowlist for false positives. |
|
f0f80d6 disables reporting of operations with inline directives. The expected result of running |
Are you thinking here of a CI job that applies the script to the codebase to ensure that there are no missing specializations or one that tests the script using the data in Beside that, do you think it is necessary to have CI run this script on Windows? After all, it’s not part of the software we want to ship in the end but a utility intended to help us (and perhaps future contributors). I mean, if supporting Windows, using some Unix compatibility layer, is easy, then I’m not against it; I just don’t want to spend much effort on making this script Windows-compatible (note that initially I intended this script to just assist me in a particular task and not even make it into the repository 🙂) |
Done in 325c20e. |
I’ve realized this in 539f06a. The wrapper script not only automatically picks the source files of the main library but also enables users to explicitly allow certain missing |
jeltsch
force-pushed
the
jeltsch/find-missing-io-specialisations
branch
2 times, most recently
from
5cc18ea to
11eb81f
Compare
jeltsch
force-pushed
the
jeltsch/find-missing-io-specialisations
branch
2 times, most recently
from
130de00 to
b018b5a
Compare
This pull requests adds a script that helps in finding missing
IOspecializations, along with some example input and output.When running the command
scripts/find-missing-io-specialisations.sh src/**/*.hson my Ubuntu 24.04 machine while being at the currentmaintip, 78a3231, I get the following output:It should be checked that the same output arises on other systems, in particular such that do not use GNU sed.