handle conditionals in duplicate module checks #7616
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
emilypi
approved these changes
Sep 4, 2021
| ++ concatMap checkTest (map snd $ condTestSuites pkg) | ||
| ++ concatMap checkBench (map snd $ condBenchmarks pkg) | ||
| where | ||
| -- the duplicate modules check is has not been thoroughly vetted for backpack |
There was a problem hiding this comment.
We can leave a TODO for that in the issues list if anyone wants to check it out.
There was a problem hiding this comment.
this was me migrating the old comment -- the current logic is no worse than before, but i didn't want to lose a potential warning here :-)
|
Tho we do need a changelog. |
Mikolaj
approved these changes
Sep 6, 2021
| checkTest = checkDups "test suite" testModules | ||
| checkBench = checkDups "benchmark" benchmarkModules | ||
| checkDups s getModules t = | ||
| let libMap = foldCondTree Map.empty |
There was a problem hiding this comment.
Why such an enormous indent? (Also, two spaces after arrow on the line below, but only one space before comments (feel free to ignore).)
jneira
added a commit
to jneira/cabal
that referenced
this pull request
Mar 21, 2022
This reverts commit ed01de6.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Resolves #4629
This is a less invasive approach than my last pass. It only factors out the duplicate module checks without touching anything else. This is a better "rough cut" approximation but still not perfect. In particular, It won't generate duplicate module warnings from a module existing on both sides of an
if/elseconditional -- however, if there are distinct conditional blocks which logically are exclusive (i.e. one is a check on a positive condition and the other a check on its negation) it won't catch that by actually model-checking the various possible outcomes.