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.

Sign up for GitHub

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

Jump to bottom

add new options for ghc 9.12 #10468

Merged
merged 1 commit into from
Oct 26, 2024
Merged

add new options for ghc 9.12 #10468

merged 1 commit into from
Oct 26, 2024
+32 −0

Conversation

geekosaur
Copy link
Collaborator

@geekosaur geekosaur commented Oct 17, 2024 •
edited
Loading

ghc 9.12 adds several new command line options, divided between
LANGUAGEs (already added), warnings, new preprocessor control options,
and compilation control options. Two options needed to be added to the
list of options requiring Int parameters.

The new options, excluding warning and language options, are:

  • -fexpose-overloaded-unfoldings
  • -fmax-forced-spec-args=N
  • -fno-expose-overloaded-unfoldings
  • -fno-object-determinism
  • -fobject-determinism
  • -fwrite-if-compression=N
  • -optCmmP…
  • -optJSP…
  • -pgmCmmP
  • -pgmJSP

As they all affect compilation and store hashes, the only necessary
change was to list the two numeric options so they will be parsed
correctly. To the best of our understanding, -pgm* and -opt*
options are already handled as a group.

Template Α: This PR modifies behaviour or interface

Include the following checklist in your PR:

  • 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!)

@geekosaur
Copy link
Collaborator Author

Does anyone know if I need to add special parsing somewhere for -pgmJSP, -optJSP, etc. somewhere? I believe we parse ghc-options contents to determine recompilation necessity and compute hashes?

@geekosaur
Copy link
Collaborator Author

Okay, already need another change, -fwrite-if-compression both changes the store (although supposedly not the meaning) and takes a numeric arg.

@geekosaur geekosaur marked this pull request as draft October 19, 2024 19:46
@geekosaur
Copy link
Collaborator Author

I suppose I won't hear anything until Monday at earliest, but with a little more understanding on my side: do we care about the new -pgmJSP and -optJSP… options? I know we support GHCJS, but I'm not sure we do anything with the new JS backend and at this point I don't see any way to support those options even as far as hashing them without a pretty major API break. That said, I also don't see what supporting them would do without some even more major changes to the build logic.

Mikolaj
Mikolaj reviewed Oct 23, 2024
View reviewed changes
Copy link
Member

@Mikolaj Mikolaj left a comment

Choose a reason for hiding this comment

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

Do any of these options look like they'd require recompilation? IIRC cabal needs to recognize those and recompile all components, not just depend on GHC managing recompilation.

@Mikolaj
Copy link
Member

Mikolaj commented Oct 23, 2024

Oh, you said "non-compile-affecting" in the ticket title. :) However, e.g., -fexpose-overloaded-unfoldings affects compilation, so I'm confused.

@geekosaur
Copy link
Collaborator Author

geekosaur commented Oct 23, 2024 •
edited
Loading

I didn't see anywhere where those went, so I assumed all options are considered to affect compilation unless specified otherwise? This is why I asked for input, and why it's still draft.

ETA: @alt-romes confirmed my understanding.

@geekosaur geekosaur force-pushed the ghc-9.12.flags branch 2 times, most recently from e35d206 to e443b73 Compare October 24, 2024 18:38
@geekosaur geekosaur marked this pull request as ready for review October 24, 2024 18:39
@geekosaur
Copy link
Collaborator Author

Per the most recent Cabal meeting (2024-10-24) this is ready to go now.

@geekosaur geekosaur changed the title add new non-compile-affecting options for ghc 9.12 add new options for ghc 9.12 Oct 24, 2024
@geekosaur
Copy link
Collaborator Author

@mergify backport 3.14

@mergify mergify bot added the ready and waiting Mergify is waiting out the cooldown period label Oct 24, 2024
@mergify Mergify
Copy link
Contributor

mergify bot commented Oct 24, 2024 •
edited
Loading

backport 3.14

✅ Backports have been created

@mergify mergify bot added the merge delay passed Applied (usually by Mergify) when PR approved and received no updates for 2 days label Oct 26, 2024
@geekosaur @Mikolaj
add new non-compile-affecting options for ghc 9.12
6578978 
- ghc 9.12 adds several new command line options, divided between
  `LANGUAGE`s (already added), warnings, new preprocessor control options,
  and compilation control options. Two options needed to be added to the
  list of options requiring `Int` parameters.

  The new options, excluding warning and language options, are:

  * `-fexpose-overloaded-unfoldings`
  * `-fmax-forced-spec-args=N`
  * `-fno-expose-overloaded-unfoldings`
  * `-fno-object-determinism`
  * `-fobject-determinism`
  * `-fwrite-if-compression=N`
  * `-optCmmP…`
  * `-optJSP…`
  * `-pgmCmmP`
  * `-pgmJSP`

  As they all affect compilation and store hashes, the only necessary
  change was to list the two numeric options so they will be parsed
  correctly. To the best of our understanding, `-pgm*` and `-opt*`
  options are already handled as a group.
@mergify mergify bot merged commit c4825fb into haskell:master Oct 26, 2024
42 checks passed
@mergify mergify bot mentioned this pull request Oct 26, 2024
Merged
5 tasks
mergify bot added a commit that referenced this pull request Oct 27, 2024
@mergify
Merge pull request #10479 from haskell/mergify/bp/3.14/pr-10468
d1fbeaf 
Backport #10468: add new options for ghc 9.12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Mikolaj Mikolaj Mikolaj left review comments

@alt-romes alt-romes alt-romes approved these changes

@ulysses4ever ulysses4ever ulysses4ever approved these changes

Assignees
No one assigned
Labels
merge delay passed Applied (usually by Mergify) when PR approved and received no updates for 2 days merge me Tell Mergify Bot to merge ready and waiting Mergify is waiting out the cooldown period
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@geekosaur @Mikolaj @ulysses4ever @alt-romes