cabal icon indicating copy to clipboard operation
cabal copied to clipboard

add the applicable new (version 9.10) GHC flags to normaliseGhcArgs

Open MangoIV opened this issue 1 year ago • 16 comments

Actionable flags are:

  • fdiagnostics-as-json (changes the format GHC outputs its diagnostics)
  • fprint-error-index-lists (changes the way GHC displays compile time)
  • fbreak-points (enables/disables break-points in GHCi)
  • dipe-stats (dumps information about which info tables have IPE information)

Please read Github PR Conventions and then fill in one of these two templates.


Template Α: This PR modifies behaviour or interface

Include the following checklist in your PR:

  • [x] Patches conform to the coding conventions.
  • [ ] Any changes that could be relevant to users have been recorded in the changelog.
  • [ ] The documentation has been updated, if necessary.
  • [ ] Manual QA notes have been included.
  • [ ] Tests have been added. (Ask for help if you don’t know how to write them! Ask for an exemption if tests are too complex for too little coverage!)

Template B: This PR does not modify behaviour or interface

E.g. the PR only touches documentation or tests, does refactorings, etc.

Include the following checklist in your PR:

  • [x] Patches conform to the coding conventions.
  • [ ] Is this a PR that fixes CI? If so, it will need to be backported to older cabal release branches (ask maintainers for directions).

MangoIV avatar May 15 '24 19:05 MangoIV

the CI tries to use base 4.20, that doesn't seem to work.

MangoIV avatar May 15 '24 19:05 MangoIV

The CI failures probably stem from github updating their GHC version, not from this PR.

Mikolaj avatar May 16 '24 14:05 Mikolaj

Let me restart the CI, which other cabal developers have fixed by now.

Mikolaj avatar May 17 '24 07:05 Mikolaj

do you have something like a "shouldNotRebuild" combinator in the test-suite to try this out?

MangoIV avatar May 17 '24 08:05 MangoIV

We have lots of tests but, strangely, I can't find any that verify that rebuilds happen as they should. I only searched for likelyi keywords. Looking up in git blame the previous PRs that touch this GHC flags code and verifying that they don't add (nor modify?) tests (naughty!) would settle the issue for me.

BTW, let me restart your CI [edit: and rebase] --- some other transient problems ensued.

Mikolaj avatar May 18 '24 08:05 Mikolaj

@mergify rebase

Mikolaj avatar May 18 '24 08:05 Mikolaj

rebase

✅ Branch has been successfully rebased

mergify[bot] avatar May 18 '24 08:05 mergify[bot]

The failure doesn’t seem to be related to my change but I’m not sure.

MangoIV avatar May 18 '24 10:05 MangoIV

Yes, the macos github runners are just flaky.

Mikolaj avatar May 18 '24 11:05 Mikolaj

tbh I'm curious why the previous GHC versions didn't add any new flags, there must have been new flags in 94, 96 and 98 that are applicable, no?

MangoIV avatar May 19 '24 14:05 MangoIV

also I can't seem to find any MRs that would adjust any tests, but I must have missed it, this dates so long back :eyes:

MangoIV avatar May 19 '24 15:05 MangoIV

In that case, it's possible we just didn't know one is supposed to add the flags and didn't understand the whispers of the ghosts of departed maintainers suggesting that we do. So there may be a several years long gap and, indeed, no tests. In which case the list of flags added in 9.10 in https://github.com/haskell/cabal/issues/9729 is not enough.

Or maybe in one of the past releases we looked at the currently added flags, decided they don't need to be included in cabal and left no trace of the deliberation in the code. This time at least we are leaving a trace in https://github.com/haskell/cabal/issues/9729.

Mikolaj avatar May 20 '24 08:05 Mikolaj

#9435, about missing unit tests for GHC options is related, isn't it?

https://github.com/haskell/cabal/blob/2dad49aae0d691896df3ad3c79c57ae55637a54e/Cabal-tests/tests/UnitTests/Distribution/Simple/Program/GHC.hs#L80-L89

$ ghc-9.10.1 --show-options | sort > 9.10.1.txt
$ ghc-9.8.2 --show-options | sort > 9.8.2.txt
$ diff -u 9.8.2.txt 9.10.1.txt
--- 9.8.2.txt	2024-05-20 06:50:18.105373180 -0400
+++ 9.10.1.txt	2024-05-20 06:50:06.597074903 -0400
...
@@ -233,6 +236,7 @@
 -fdefer-out-of-scope-variables
 -fdefer-typed-holes
 -fdefer-type-errors
+-fdiagnostics-as-json
 -fdiagnostics-color=always
 -fdiagnostics-color=auto
 -fdiagnostics-color=never
 ...

philderbeast avatar May 20 '24 10:05 philderbeast

In that case, it's possible we just didn't know one is supposed to add the flags and didn't understand the whispers of the ghosts of departed maintainers suggesting that we do. So there may be a several years long gap and, indeed, no tests. In which case the list of flags added in 9.10 in https://github.com/haskell/cabal/issues/9729 is not enough.

Or maybe in one of the past releases we looked at the currently added flags, decided they don't need to be included in cabal and left no trace of the deliberation in the code. This time at least we are leaving a trace in https://github.com/haskell/cabal/issues/9729.

@Mikolaj do you want to me to dig though the flags for the remaining GHC releases, 9.2, 9.4, 9.6, 9.8?

MangoIV avatar May 20 '24 14:05 MangoIV

@Mikolaj do you want to me to dig though the flags for the remaining GHC releases, 9.2, 9.4, 9.6, 9.8?

@MangoIV: if that's not too much to ask, that would be very helpful.

Mikolaj avatar May 20 '24 19:05 Mikolaj

Alright no of course it’s not too much to ask, I just wanted to know as I’m new 😅

MangoIV avatar May 20 '24 19:05 MangoIV

The diff between GHC 8.10 (last time cabal updated the file) and GHC 9.10 (latest)

17a18,19
> -ddisable-js-c-sources
> -ddisable-js-minifier
20d21
< -ddump-asm-expanded
27a29
> -ddump-c-backend
35a38
> -ddump-cmm-opt
42a46
> -ddump-cmm-thread-sanitizer
45a50,51
> -ddump-cpranal
> -ddump-cpr-signatures
49a56,57
> -ddump-dmdanal
> -ddump-dmd-signatures
53a62
> -ddump-faststrings
54a64,65
> -ddump-float-in
> -ddump-float-out
55a67
> -ddump-full-laziness
57a70
> -ddump-hie
60a74
> -ddump-js
61a76,77
> -ddump-late-cc
> -ddump-liberate-case
83a100
> -ddump-spec-constr
84a102
> -ddump-static-argument-transformation
85a104
> -ddump-stg-cg
86a106,107
> -ddump-stg-from-core
> -ddump-stg-tags
96a118
> -ddump-verbose-inlinings
98d119
< -ddump-vt-trace
106a128
> -dipe-stats
108a131,133
> -dkeep-comments
> -dlinear-core-lint
> -dlint
113a139,140
> -dno-suppress-coercion-types
> -dno-suppress-core-sizes
117a145
> -dno-suppress-stg-reps
137a166,167
> -dsuppress-coercion-types
> -dsuppress-core-sizes
141a172
> -dsuppress-stg-reps
148a180
> -dtag-inference-checks
159a192
> -dynohi
174d206
< -fast-llvm
179a212
> -fbreak-points
181a215
> -fbyte-code-and-object-code
185c219,220
< -fcatch-bottoms
---
> -fcatch-nonexhaustive-cases
> -fcheck-prim-bounds
186a222
> -fcmm-control-flow
188a225,227
> -fcmm-static-pred
> -fcmm-thread-sanitizer
> -fcompact-unwind
190a230
> -fcore-constant-folding
198a239
> -fdiagnostics-as-json
204a246
> -fdistinct-constructor-tables
205a248,249
> -fdmd-unbox-width
> -fdo-clever-arg-eta-expansion
207a252
> -fdump-with-ways
215a261
> -fexpose-internal-symbols
218a265,266
> -ffamily-application-cache
> -ffast-pap-calls
221d268
< -fflat-cache
232a280
> -fgivens-expansion-fuel
243a292,296
> -finfo-table-map
> -finfo-table-map-with-fallback
> -finfo-table-map-with-stack
> -finline-generics
> -finline-generics-aggressively
244a298
> -fkeep-auto-rules
247,248d300
< -fkill-absence
< -fkill-one-shot
252a305
> -flink-rts
254a308,309
> -flocal-float-out
> -flocal-float-out-top-level
269d323
< -fmono-pat-binds
279a334
> -fno-break-points
280a336
> -fno-byte-code-and-object-code
284c340,342
< -fno-catch-bottoms
---
> -fno-catch-nonexhaustive-cases
> -fno-check-prim-bounds
> -fno-cmm-control-flow
286a345,346
> -fno-cmm-static-pred
> -fno-cmm-thread-sanitizer
287a348,349
> -fno-compact-unwind
> -fno-core-constant-folding
295a358
> -fno-diagnostics-as-json
298a362
> -fno-distinct-constructor-tables
299a364
> -fno-do-clever-arg-eta-expansion
301a367
> -fno-dump-with-ways
309a376
> -fno-expose-internal-symbols
312a380,381
> -fno-family-application-cache
> -fno-fast-pap-calls
315d383
< -fno-flat-cache
333a402,406
> -fno-info-table-map
> -fno-info-table-map-with-fallback
> -fno-info-table-map-with-stack
> -fno-inline-generics
> -fno-inline-generics-aggressively
334a408
> -fno-keep-auto-rules
337,338d410
< -fno-kill-absence
< -fno-kill-one-shot
342a415,417
> -fno-link-rts
> -fno-local-float-out
> -fno-local-float-out-top-level
349d423
< -fno-mono-pat-binds
354a429
> -fno-orig-thunk-info
357a433,434
> -fno-polymorphic-specialisation
> -fno-prefer-byte-code
366a444
> -fno-print-redundant-promotion-ticks
371a450,454
> -fno-prof-late
> -fno-prof-late-inline
> -fno-prof-late-overloaded
> -fno-prof-late-overloaded-calls
> -fno-prof-manual
381a465
> -fno-show-error-context
401a486
> -fno-specialise-incoherents
403a489
> -fno-split-sections
413a500
> -fno-unoptimized-core-for-interpreter
418a506
> -fno-worker-wrapper-cbv
419a508
> -fno-write-if-simplified-core
425a515
> -forig-thunk-info
430a521
> -fplugin-library
432a524,525
> -fpolymorphic-specialisation
> -fprefer-byte-code
435a529,531
> -fprint-error-index-links=always
> -fprint-error-index-links=auto
> -fprint-error-index-links=never
441a538
> -fprint-redundant-promotion-ticks
449a547
> -fprof-callers
450a549,554
> -fprof-late
> -fprof-late-inline
> -fprof-late-overloaded
> -fprof-late-overloaded-calls
> -fprof-manual
> -fqcs-expansion-fuel
462a567
> -fshow-error-context
485a591
> -fspecialise-incoherents
487a594
> -fsplit-sections
502a610,611
> -funfolding-case-scaling
> -funfolding-case-threshold
507a617
> -funoptimized-core-for-interpreter
512a623
> -fwanteds-expansion-fuel
514a626
> -fworker-wrapper-cbv
515a628
> -fwrite-if-simplified-core
524a638
> -hidden-module
542a657
> -jsem
568a684,685
> --merge-objs
> -mfma
579a697
> -no-haddock
604a723
> -optlas
622c741
< -pgmdll
---
> -pgmcxx
627a747
> -pgmlas
629d748
< -pgmlibtool
631a751
> -pgml-supports-no-pie
649d768
< --print-ld-flags
663a783
> -reexported-module
671a792
> --run
677,678c798
< -smp
< -split-objs
---
> -single-threaded
687c807
< -this-package-key
---
> -this-package-name
691a812
> -ticky-ap-thunk
693a815
> -ticky-tag-checks
696a819
> -unit
706a830
> -Wambiguous-fields
707a832
> -Wbadly-staged-types
710a836
> -Wdata-kinds-tc
711a838
> -Wdefaulted-exception-context
715c842
< -Wdeprecations
---
> -Wdeprecated-type-abstractions
724a852
> -Werror=all
727a856
> -Werror=ambiguous-fields
728a858
> -Werror=badly-staged-types
731a862,864
> -Werror=data-kinds-tc
> -Werror=default
> -Werror=defaulted-exception-context
735c868
< -Werror=deprecations
---
> -Werror=deprecated-type-abstractions
743a877,881
> -Werror=everything
> -Werror=extended-warnings
> -Werror=extra
> -Werror=forall-identifier
> -Werror=gadt-mono-local-binds
746a885
> -Werror=implicit-lift
747a887
> -Werror=implicit-rhs-quantification
748a889
> -Werror=incomplete-export-warnings
749a891
> -Werror=incomplete-record-selectors
751a894
> -Werror=inconsistent-flags
753a897,899
> -Werror=invalid-haddock
> -Werror=loopy-superclass-solve
> -Werror=misplaced-pragmas
757a904
> -Werror=missing-exported-pattern-synonym-signatures
763a911
> -Werror=missing-kind-signatures
768a917,918
> -Werror=missing-poly-kind-signatures
> -Werror=missing-role-annotations
776a927,928
> -Werror=operator-whitespace
> -Werror=operator-whitespace-ext-conflict
782a935
> -Werror=redundant-bang-patterns
784a938
> -Werror=redundant-strictness-flags
790a945
> -Werror=term-variable-capture
793a949,950
> -Werror=type-equality-out-of-scope
> -Werror=type-equality-requires-operators
794a952
> -Werror=unicode-bidirectional-format-characters
800a959
> -Werror=unused-binds
813a973
> -Wextended-warnings
814a975,976
> -Wforall-identifier
> -Wgadt-mono-local-binds
817a980
> -Wimplicit-lift
818a982
> -Wimplicit-rhs-quantification
819a984
> -Wincomplete-export-warnings
820a986
> -Wincomplete-record-selectors
822a989
> -Winconsistent-flags
824a992
> -Winvalid-haddock
825a994,995
> -Wloopy-superclass-solve
> -Wmisplaced-pragmas
829a1000
> -Wmissing-exported-pattern-synonym-signatures
835a1007
> -Wmissing-kind-signatures
840a1013,1014
> -Wmissing-poly-kind-signatures
> -Wmissing-role-annotations
849a1024
> -Wno-ambiguous-fields
850a1026
> -Wno-badly-staged-types
853a1030
> -Wno-data-kinds-tc
854a1032
> -Wno-defaulted-exception-context
858c1036
< -Wno-deprecations
---
> -Wno-deprecated-type-abstractions
866a1045
> -Wno-error=all
869a1049
> -Wno-error=ambiguous-fields
870a1051
> -Wno-error=badly-staged-types
873a1055,1057
> -Wno-error=data-kinds-tc
> -Wno-error=default
> -Wno-error=defaulted-exception-context
877c1061
< -Wno-error=deprecations
---
> -Wno-error=deprecated-type-abstractions
885a1070,1074
> -Wno-error=everything
> -Wno-error=extended-warnings
> -Wno-error=extra
> -Wno-error=forall-identifier
> -Wno-error=gadt-mono-local-binds
888a1078
> -Wno-error=implicit-lift
889a1080
> -Wno-error=implicit-rhs-quantification
890a1082
> -Wno-error=incomplete-export-warnings
891a1084
> -Wno-error=incomplete-record-selectors
893a1087
> -Wno-error=inconsistent-flags
895a1090,1092
> -Wno-error=invalid-haddock
> -Wno-error=loopy-superclass-solve
> -Wno-error=misplaced-pragmas
899a1097
> -Wno-error=missing-exported-pattern-synonym-signatures
905a1104
> -Wno-error=missing-kind-signatures
910a1110,1111
> -Wno-error=missing-poly-kind-signatures
> -Wno-error=missing-role-annotations
918a1120,1121
> -Wno-error=operator-whitespace
> -Wno-error=operator-whitespace-ext-conflict
924a1128
> -Wno-error=redundant-bang-patterns
926a1131
> -Wno-error=redundant-strictness-flags
932a1138
> -Wno-error=term-variable-capture
935a1142,1143
> -Wno-error=type-equality-out-of-scope
> -Wno-error=type-equality-requires-operators
936a1145
> -Wno-error=unicode-bidirectional-format-characters
942a1152
> -Wno-error=unused-binds
955a1166
> -Wno-extended-warnings
956a1168,1169
> -Wno-forall-identifier
> -Wno-gadt-mono-local-binds
959a1173
> -Wno-implicit-lift
960a1175
> -Wno-implicit-rhs-quantification
961a1177
> -Wno-incomplete-export-warnings
962a1179
> -Wno-incomplete-record-selectors
964a1182
> -Wno-inconsistent-flags
966a1185,1187
> -Wno-invalid-haddock
> -Wno-loopy-superclass-solve
> -Wno-misplaced-pragmas
970a1192
> -Wno-missing-exported-pattern-synonym-signatures
976a1199
> -Wno-missing-kind-signatures
981a1205,1206
> -Wno-missing-poly-kind-signatures
> -Wno-missing-role-annotations
992a1218,1219
> -Wno-operator-whitespace
> -Wno-operator-whitespace-ext-conflict
998a1226
> -Wno-redundant-bang-patterns
1000a1229
> -Wno-redundant-strictness-flags
1007a1237
> -Wno-term-variable-capture
1010a1241,1242
> -Wno-type-equality-out-of-scope
> -Wno-type-equality-requires-operators
1011a1244
> -Wno-unicode-bidirectional-format-characters
1030a1264,1266
> -Woperator-whitespace
> -Woperator-whitespace-ext-conflict
> -working-dir
1036a1273
> -Wredundant-bang-patterns
1038a1276
> -Wredundant-strictness-flags
1044a1283
> -Wterm-variable-capture
1047a1287,1288
> -Wtype-equality-out-of-scope
> -Wtype-equality-requires-operators
1048a1290
> -Wunicode-bidirectional-format-characters
1066a1309
> -Wwarn=all
1069a1313
> -Wwarn=ambiguous-fields
1070a1315
> -Wwarn=badly-staged-types
1073a1319,1321
> -Wwarn=data-kinds-tc
> -Wwarn=default
> -Wwarn=defaulted-exception-context
1077c1325
< -Wwarn=deprecations
---
> -Wwarn=deprecated-type-abstractions
1085a1334,1338
> -Wwarn=everything
> -Wwarn=extended-warnings
> -Wwarn=extra
> -Wwarn=forall-identifier
> -Wwarn=gadt-mono-local-binds
1088a1342
> -Wwarn=implicit-lift
1089a1344
> -Wwarn=implicit-rhs-quantification
1090a1346
> -Wwarn=incomplete-export-warnings
1091a1348
> -Wwarn=incomplete-record-selectors
1093a1351
> -Wwarn=inconsistent-flags
1096a1355,1357
> -Wwarn=invalid-haddock
> -Wwarn=loopy-superclass-solve
> -Wwarn=misplaced-pragmas
1100a1362
> -Wwarn=missing-exported-pattern-synonym-signatures
1106a1369
> -Wwarn=missing-kind-signatures
1111a1375,1376
> -Wwarn=missing-poly-kind-signatures
> -Wwarn=missing-role-annotations
1119a1385,1386
> -Wwarn=operator-whitespace
> -Wwarn=operator-whitespace-ext-conflict
1125a1393
> -Wwarn=redundant-bang-patterns
1127a1396
> -Wwarn=redundant-strictness-flags
1133a1403
> -Wwarn=term-variable-capture
1136a1407,1408
> -Wwarn=type-equality-out-of-scope
> -Wwarn=type-equality-requires-operators
1137a1410
> -Wwarn=unicode-bidirectional-format-characters
1143a1417
> -Wwarn=unused-binds
1172a1447
> -XDeepSubsumption
1193a1469,1470
> -XExtendedLiterals
> -XFieldSelectors
1202c1479,1480
< -XGenerics
---
> -XGHC2021
> -XGHC2024
1216a1495
> -XLexicalNegation
1217a1497,1498
> -XLinearTypes
> -XListTuplePuns
1220d1500
< -XMonadFailDesugaring
1223d1502
< -XMonoPatBinds
1244a1524
> -XNoDeepSubsumption
1265a1546,1547
> -XNoExtendedLiterals
> -XNoFieldSelectors
1274d1555
< -XNoGenerics
1286a1568
> -XNoLexicalNegation
1287a1570,1571
> -XNoLinearTypes
> -XNoListTuplePuns
1290d1573
< -XNoMonadFailDesugaring
1293d1575
< -XNoMonoPatBinds
1307a1590,1591
> -XNoOverloadedRecordDot
> -XNoOverloadedRecordUpdate
1318a1603
> -XNoQualifiedDo
1328a1614
> -XNoRequiredTypeArguments
1341a1628
> -XNoTypeAbstractions
1342a1630
> -XNoTypeData
1352a1641
> -XNoUnliftedDatatypes
1362a1652,1653
> -XOverloadedRecordDot
> -XOverloadedRecordUpdate
1373a1665
> -XQualifiedDo
1383a1676
> -XRequiredTypeArguments
1398a1692
> -XTypeAbstractions
1399a1694
> -XTypeData
1409a1705
> -XUnliftedDatatypes

MangoIV avatar May 31 '24 09:05 MangoIV

For future reference: to compare the flags between ghc versios, ghc offers the handy command --show-options which outputs all the options except the ones for custom warnings (which are not relevant for these PRs).

MangoIV avatar May 31 '24 09:05 MangoIV

things I'm not sure about:

  • -fprefer-bytecode: it looks like this can be included if we're sure that the results from evaluating the bytecode TH splices and linked code TH splices are identical, but I don't know if that's true.

MangoIV avatar May 31 '24 09:05 MangoIV

I think I have gone though all the applicable extensions. Should I add any specific tests for this?

MangoIV avatar May 31 '24 12:05 MangoIV

-fprefer-bytecode: it looks like this can be included if we're sure that the results from evaluating the bytecode TH splices and linked code TH splices are identical, but I don't know if that's true.

Strangely, I can't find this option in https://downloads.haskell.org/ghc/latest/docs/users_guide. I can only find a GHC MR where something to do with this option is fixed. If it's that obscure (and not due to a typo or whatever other silly reason), maybe let's just ignore it and not put it into cabal code (or only in a comment saying, it's not in the manual so we do nothing).

I think I have gone though all the applicable extensions. Should I add any specific tests for this?

Thanks a lot. Yes, any simple test would be great, even a smoke test of some kind.

cabal-install with GHC 9.8 and newer must not list the cabal equivalent of the GHC split-objs flag, before GHC 9.8 it must list it

List where? What's the equivalent precisely?

Mikolaj avatar May 31 '24 22:05 Mikolaj

That one seems like it might be a little complicated, because it's supposed to have been preferring --split-sections for some time now. (Except that that doesn't work on MacOS or Windows. MacOS has a sort-of-equivalent mechanism which GHC always uses, though, AIUI.)

geekosaur avatar May 31 '24 22:05 geekosaur

We are finalizing the 3.12.1.0 cabal-install release and closing the list of backports. Is this PR close to done?

Mikolaj avatar Jun 07 '24 21:06 Mikolaj

It needs the smoke tests, I can do that today.

MangoIV avatar Jun 08 '24 07:06 MangoIV

I would also be happy to have a second pair of eyes looking at the extensions.

MangoIV avatar Jun 08 '24 07:06 MangoIV

Actually, which extensions?

Mikolaj avatar Jun 08 '24 10:06 Mikolaj

I have answered in the matrix channel :)

MangoIV avatar Jun 08 '24 11:06 MangoIV

Manual CI typos(?):

  • -fshow-error-context and -funoptimized-core-for-interpreter are missing their leading fs
  • -fprint-error-index-lists is actually -fprint-error-index-links and requires a parameter

geekosaur avatar Jun 09 '24 22:06 geekosaur

With those corrected, it works here.

geekosaur avatar Jun 09 '24 22:06 geekosaur

@MangoIV this PR is currently marked as draft by the author. To move forward, we need this changed.

ulysses4ever avatar Jun 10 '24 01:06 ulysses4ever