Skip to content
This repository was archived by the owner on Jan 2, 2021. It is now read-only.

Fix the build for ghc-8.8.2. #335

Merged
merged 2 commits into from
Jan 21, 2020
Merged

Fix the build for ghc-8.8.2. #335

Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
d83512f
Fix the build for ghc-8.8.2.
robrix Jan 20, 2020
e9e01c4
Match the single-element list first.
robrix Jan 21, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 8 additions & 2 deletions src/Development/IDE/GHC/CPP.hs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,9 @@ import Module
import DynFlags
import Panic
import FileCleanup
#if MIN_GHC_API_VERSION(8,8,0)
#if MIN_GHC_API_VERSION(8,8,2)
import LlvmCodeGen (LlvmVersion, llvmVersionList)
#elif MIN_GHC_API_VERSION(8,8,0)
import LlvmCodeGen (LlvmVersion (..))
#endif

Expand Down Expand Up @@ -139,7 +141,11 @@ getBackendDefs :: DynFlags -> IO [String]
getBackendDefs dflags | hscTarget dflags == HscLlvm = do
llvmVer <- figureLlvmVersion dflags
return $ case llvmVer of
#if MIN_GHC_API_VERSION(8,8,0)
#if MIN_GHC_API_VERSION(8,8,2)
Just v
| m:n:_ <- llvmVersionList v -> [ "-D__GLASGOW_HASKELL_LLVM__=" ++ format (m, n) ]
| n:_ <- llvmVersionList v -> [ "-D__GLASGOW_HASKELL_LLVM__=" ++ format (n, 0) ]
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don’t have any projects which use LLVM, so I’d appreciate it if someone could double-check that this is doing what they expect.

Note that despite llvmVersionList returning [Int], these branches are in fact exhaustive (under ghc 8.8.2), because LlvmVersion contains a NonEmpty list and converts it to a list otherwise unmodified. So we will always have at least a major version, and thus this will not silently fail (modulo future changes to ghc’s API).

#elif MIN_GHC_API_VERSION(8,8,0)
Copy link
Collaborator

Choose a reason for hiding this comment

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

This matches the way this is written in GHC and is a bit easier to read imho: https://gitlab.haskell.org/ghc/ghc/blob/ghc-8.8/compiler/main/DriverPipeline.hs#L2174

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Aha, good call 👍

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Shame they didn’t export that.

Copy link
Collaborator

Choose a reason for hiding this comment

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

The whole doCpp is exported from GHC 8.10 and above, so the whole thing can disappear.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@ndmitchell Do you mean that we should do that in this PR? Cos I’m not really set up to test against 8.10.* just now.

Copy link
Collaborator

Choose a reason for hiding this comment

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

No - we should leave it as it is for now. Once 8.10 is actually out we should do some conditional stuff - more just pointing out there is a long term plan for getting rid of it all.

Copy link
Collaborator

Choose a reason for hiding this comment

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

No, we need to support older versions so this can’t go away completely in particular not the code for 8.8.2. We can however, use the definition from GHC in 8.10 but let’s please keep 8.10 support for a separate PR. I expect that there are a bunch of other issues anyway.

Just (LlvmVersion n) -> [ "-D__GLASGOW_HASKELL_LLVM__=" ++ format (n,0) ]
Just (LlvmVersionOld m n) -> [ "-D__GLASGOW_HASKELL_LLVM__=" ++ format (m,n) ]
#else
Expand Down