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

GHC 8.10.3, Cabal 2.2 inline-c-cpp cleanup #121

Merged
merged 10 commits into from Jan 2, 2021
Merged

GHC 8.10.3, Cabal 2.2 inline-c-cpp cleanup #121

merged 10 commits into from Jan 2, 2021

Conversation

roberth
Copy link
Collaborator

@roberth roberth commented Nov 23, 2020 •
edited

New, cleaner attempt to support GHC 8.10.
Previous reasoning was flawed.

This PR cleans up a number of things around inline-c-cpp.
It does not support LTS 11 (GHC 8.2). LTS 11 users can of course still use the inline-c-cpp from the snapshot.
It also does not attempt to set C++ compiler options on GHCs older than 8.10. This does not affect user packages because those still use the cc-options hack. They can do the same cleanup when they see fit.

This also means that we don't set c++11 for inline-c-cpp itself. This should be ok, because

GCC, LLVM, and VC++ have been designed to enable binary compatibility across different versions of the C++ standard. This is not really a requirement of the standard itself though.

-- https://stackoverflow.com/questions/46746878/is-it-safe-to-link-c17-c14-and-c11-objects/49118876~

bitonic
bitonic requested changes Nov 23, 2020
View reviewed changes
inline-c-cpp/inline-c-cpp.cabal Outdated Show resolved Hide resolved
inline-c-cpp/inline-c-cpp.cabal Show resolved Hide resolved
@roberth roberth marked this pull request as draft November 23, 2020 13:07
@roberth roberth marked this pull request as ready for review November 23, 2020 13:53
@roberth roberth requested a review from bitonic November 23, 2020 13:53
bitonic
bitonic requested changes Nov 23, 2020
View reviewed changes
inline-c-cpp/inline-c-cpp.cabal Outdated Show resolved Hide resolved
@roberth
Drop LTS 11 / GHC 8.2
0bf8467 
LTS 11 comes with Cabal < 2.2, which doesn't support cxx-options.
@roberth roberth force-pushed the cabal-2.2 branch 3 times, most recently from a123cb3 to 493a2f9 Compare November 26, 2020 22:28
@roberth
Copy link
Collaborator Author

roberth commented Nov 26, 2020

Previously my manual testing has been distorted by unclean builds.
Cabal doesn't rebuild C/C++ files when options change.
This doesn't affect builds in Nix or on CI, but it has made the hacking on this PR needlessly puzzling and frustrating.

roberth
roberth commented Nov 26, 2020
View reviewed changes
inline-c-cpp/inline-c-cpp.cabal
if os(darwin)
ghc-options: -pgmc=clang++
extra-libraries: stdc++
cc-options: -Wall -Werror -optc-xc++ -std=c++11
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Restoring -Werror is feasible but may require duplicating a large part of common cxx-opts

@roberth
Copy link
Collaborator Author

roberth commented Nov 27, 2020

This is now blocked on a Cabal update to fix haskell/cabal#6421. This is available in Cabal 3.2.1.0.
Stackage Nightly currently comes with Cabal 3.2.0.0.

@roberth roberth mentioned this pull request Nov 27, 2020
Closed
@bitonic
Copy link
Collaborator

bitonic commented Nov 27, 2020

Thanks so much for all the work / research -- let's merge when things are in working order.

@roberth
inline-c-cpp: Remove -optc-xc++, clean up -std=c++11
8ca7e75 
-xc++ must be provided on the command line before listing the
input files, which Cabal does not do. We rely on -std=c++11
(or similar) to switch to C++ on GHC < 8.10 with GCC.

We also remove a redundant -std=c++11 because it is already
provided in the common stanza.
@roberth
Try macOS
27ac296
@roberth
inline-c-cpp: Only pass -Werror to GHC's C++ when >= 8.10
9621c2c
@roberth
ci: Disable gsl example on macOS
f17c6f8 
The error:

inline-c            > configure (lib + exe + test)
inline-c            > Configuring inline-c-0.9.1.3...
inline-c            > Cabal-simple_mPHDZzAJ_3.0.1.0_ghc-8.8.4: Missing dependencies on foreign
inline-c            > libraries:
inline-c            > * Missing (or bad) C libraries: gsl, gslcblas
@roberth roberth changed the title Cabal 2.2 inline-c-cpp cleanup GHC 8.10.3, Cabal 2.2 inline-c-cpp cleanup Jan 2, 2021
@roberth roberth requested a review from bitonic January 2, 2021 11:02
@roberth
Copy link
Collaborator Author

roberth commented Jan 2, 2021

This is ready for review.

As a bonus, instead of only a minimal stack.yaml, macOS now also runs for GHC 8.6 - 8.10. It now also builds the C++ example instead of none.

@bitonic
Copy link
Collaborator

bitonic commented Jan 2, 2021

@roberth thanks!

@bitonic bitonic merged commit a4d8333 into fpco:master Jan 2, 2021
@bitonic
Copy link
Collaborator

bitonic commented Jan 2, 2021

Released as 0.9.1.4.

@roberth
Copy link
Collaborator Author

roberth commented Jan 18, 2021

@bitonic Most of these changes are in inline-c-cpp rather than inline-c. Could you release inline-c-cpp as well?

@bitonic
Copy link
Collaborator

bitonic commented Jan 18, 2021

@roberth just did so, sorry about the oversight.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@bitonic bitonic Awaiting requested review from bitonic

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@roberth @bitonic