Satisfy -Werror=unused-top-binds

[philderbeast]

Satisfy the build with this (uncommited) change:

git diff
diff --git a/cabal.project b/cabal.project
index d0b2fbabc..c0695f378 100644
--- a/cabal.project
+++ b/cabal.project
@@ -20,4 +20,4 @@ constraints: rere -rere-cfg
 constraints: these -assoc
 
 program-options
-  ghc-options: -fno-ignore-asserts
+  ghc-options: -fno-ignore-asserts -Werror=unused-top-binds

• Patches conform to the coding conventions.

We can pick up the deleted top binds of Cabal-tests/tests/UnitTests/Distribution/Simple/Program/GHC.hs in #9435.

[Mikolaj]

Could you elaborate? Does it break CI or your local tests? Only with some GHCs?

[philderbeast]

The unused top binds are dead code.

[Mikolaj]

Oh, I see! If you do the change cited in the description, then compilation would error and your PR fixes the error by removing dead code. Did I get it right?

And the dead code is worrying, because it suggests some old changes have been misapplied or corrupted, right? Should we maybe not remove the dead code until we investigate and either plug it back in or decide it's unneeded?

[ulysses4ever]

Took me quite a while to understand the motivation just by reading the description, unfortunately.

For the context: we already use -Werror for the main packages in this repo -- see cabal.project.validate. This patch explores a partial -Werror for a test package. So, the natural question is: why actually not go further and add that partial -Werror for the test package in cabal.project.validate?..

[philderbeast]

@ulysses4ever at the time I didn't want to delve into the various projects and their purpose so instead I showed how I found there was dead code.

$ tree -P 'cabal.project*' --prune -L 1
.
├── cabal.project
├── cabal.project.buildinfo
├── cabal.project.coverage
├── cabal.project.doctest
├── cabal.project.latest-ghc
├── cabal.project.libonly
├── cabal.project.meta
├── cabal.project.release
├── cabal.project.validate
├── cabal.project.validate.libonly
└── cabal.project.weeder

1 directory, 11 files

The cabal.project.validate project has -Werror for some of its packages but this won't trigger -Werror=unused-top-binds. We'd need to flag -Wunused-top-binds or -Wunused-binds or -Wextra to include those warnings. We don't set a warning level in that project so will be getting warnings included in -Wdefault (from GHC's Warnings and sanity-checking).

[philderbeast]

Label merge+no rebase is necessary when the pull request is from an organisation.

[philderbeast]

For the context: we already use -Werror for the main packages in this repo -- see cabal.project.validate. This patch explores a partial -Werror for a test package. So, the natural question is: why actually not go further and add that partial -Werror for the test package in cabal.project.validate?..

Thanks for suggesting this @ulysses4ever. I reviewed the projects with #9565 but have not yet got around to changing the warning levels (except using ghc-options: -Werror from program-options instead of doing it more explicitly per package).