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

fix incomplete-uni-patterns warnings #1176

Merged

Conversation

peterbecich
Copy link
Member

@peterbecich peterbecich commented Feb 4, 2023

the flake.nix build fails on these warnings: #1154

No effective changes in this PR

the `flake.nix` build fails on these warnings: haskell#1154
exes/Main.hs Outdated
checkHostURI defaults Nothing port = do
let guessURI = confHostUri defaults
Just authority = uriAuthority guessURI
case uriAuthority guessURI of
Nothing -> fail "No URI Authority"
Copy link
Member Author

@peterbecich peterbecich Feb 4, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this new fail is the only change in this PR which might have some effect, is this acceptable?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not simply disable the warning also in this case?
Is there evidence that this Just match can actually fail?
NB: Ideally unexpected failing matches would come with source location (HasCallStack).

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No problem, reverted the changes and applied the pragma, like the other files

It was the easiest of the warnings to fix, and using fail is consistent with the rest of the file IMO

There is no evidence the Just match can actually fail

Copy link
Member

@andreasabel andreasabel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My general strategy would be to turn off the incomplete-uni-patterns warning, unless there is evidence that the match can actually fail.
(This is the conservative approach, trusting that the original code was correct.)

exes/Main.hs Outdated
checkHostURI defaults Nothing port = do
let guessURI = confHostUri defaults
Just authority = uriAuthority guessURI
case uriAuthority guessURI of
Nothing -> fail "No URI Authority"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not simply disable the warning also in this case?
Is there evidence that this Just match can actually fail?
NB: Ideally unexpected failing matches would come with source location (HasCallStack).

@peterbecich peterbecich changed the title fix incomplete-uni-patterns warning, ignore some others fix incomplete-uni-patterns warning Feb 25, 2023
@peterbecich peterbecich changed the title fix incomplete-uni-patterns warning fix incomplete-uni-patterns warnings Feb 25, 2023
@andreasabel andreasabel merged commit 70b2934 into haskell:master Feb 26, 2023
@andreasabel
Copy link
Member

Thanks @peterbecich !

@peterbecich peterbecich deleted the incomplete-uni-patterns-warnings branch February 27, 2023 05:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants