Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
4 changes: 4 additions & 0 deletions Cabal/Cabal.cabal
Original file line number Diff line number Diff line change
Expand Up @@ -75,12 +75,16 @@ extra-source-files:
tests/ParserTests/regressions/Octree-0.5.format
tests/ParserTests/regressions/bad-glob-syntax.cabal
tests/ParserTests/regressions/bad-glob-syntax.check
tests/ParserTests/regressions/cc-options-with-optimization.cabal
tests/ParserTests/regressions/cc-options-with-optimization.check
tests/ParserTests/regressions/common.cabal
tests/ParserTests/regressions/common.expr
tests/ParserTests/regressions/common.format
tests/ParserTests/regressions/common2.cabal
tests/ParserTests/regressions/common2.expr
tests/ParserTests/regressions/common2.format
tests/ParserTests/regressions/cxx-options-with-optimization.cabal
tests/ParserTests/regressions/cxx-options-with-optimization.check
tests/ParserTests/regressions/elif.cabal
tests/ParserTests/regressions/elif.expr
tests/ParserTests/regressions/elif.format
Expand Down
5 changes: 5 additions & 0 deletions Cabal/ChangeLog.md
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,11 @@
`Distribution.Simple.Glob` and `FileGlob` has been made abstract.

(#5284, #3178, et al.)
* Fixed `cxx-options` and `cxx-sources` buildinfo fields for
separate compilation of C++ source files to correctly build and link
non-library components (#5309).
* Reduced warnings generated by hsc2hs and c2hs when `cxx-options` field
is present in a component.

----

Expand Down
34 changes: 20 additions & 14 deletions Cabal/Distribution/PackageDescription/Check.hs
Original file line number Diff line number Diff line change
Expand Up @@ -165,6 +165,7 @@ checkConfiguredPackage pkg =
++ checkSourceRepos pkg
++ checkGhcOptions pkg
++ checkCCOptions pkg
++ checkCxxOptions pkg
++ checkCPPOptions pkg
++ checkPaths pkg
++ checkCabalVersion pkg
Expand Down Expand Up @@ -960,17 +961,23 @@ checkGhcOptions pkg =
disable e = Just (DisableExtension e)

checkCCOptions :: PackageDescription -> [PackageCheck]
checkCCOptions pkg =
checkCCOptions = checkCLikeOptions "C" "cc-options" ccOptions

checkCxxOptions :: PackageDescription -> [PackageCheck]
checkCxxOptions = checkCLikeOptions "C++" "cxx-options" cxxOptions

checkCLikeOptions :: String -> String -> (BuildInfo -> [String]) -> PackageDescription -> [PackageCheck]
checkCLikeOptions label prefix accessor pkg =
catMaybes [

checkAlternatives "cc-options" "include-dirs"
[ (flag, dir) | flag@('-':'I':dir) <- all_ccOptions ]
checkAlternatives prefix "include-dirs"
[ (flag, dir) | flag@('-':'I':dir) <- all_cLikeOptions ]

, checkAlternatives "cc-options" "extra-libraries"
[ (flag, lib) | flag@('-':'l':lib) <- all_ccOptions ]
, checkAlternatives prefix "extra-libraries"
[ (flag, lib) | flag@('-':'l':lib) <- all_cLikeOptions ]

, checkAlternatives "cc-options" "extra-lib-dirs"
[ (flag, dir) | flag@('-':'L':dir) <- all_ccOptions ]
, checkAlternatives prefix "extra-lib-dirs"
[ (flag, dir) | flag@('-':'L':dir) <- all_cLikeOptions ]

, checkAlternatives "ld-options" "extra-libraries"
[ (flag, lib) | flag@('-':'l':lib) <- all_ldOptions ]
Expand All @@ -980,19 +987,18 @@ checkCCOptions pkg =

, checkCCFlags [ "-O", "-Os", "-O0", "-O1", "-O2", "-O3" ] $
PackageDistSuspicious $
"'cc-options: -O[n]' is generally not needed. When building with "
++ " optimisations Cabal automatically adds '-O2' for C code. "
++ "Setting it yourself interferes with the --disable-optimization "
++ "flag."
"'"++prefix++": -O[n]' is generally not needed. When building with "
++ " optimisations Cabal automatically adds '-O2' for "++label++" code. "
++ "Setting it yourself interferes with the --disable-optimization flag."
Copy link
Contributor

Choose a reason for hiding this comment

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

Cabal/tests/ParserTests has some tests for warnings from check, which you might want to add to.

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 added the following test files to ensure that the check provides correct warnings:
regressions/cc-options-with-optimization
regressions/cxx-options-with-optimization

]

where all_ccOptions = [ opts | bi <- allBuildInfo pkg
, opts <- ccOptions bi ]
where all_cLikeOptions = [ opts | bi <- allBuildInfo pkg
, opts <- accessor bi ]
all_ldOptions = [ opts | bi <- allBuildInfo pkg
, opts <- ldOptions bi ]

checkCCFlags :: [String] -> PackageCheck -> Maybe PackageCheck
checkCCFlags flags = check (any (`elem` flags) all_ccOptions)
checkCCFlags flags = check (any (`elem` flags) all_cLikeOptions)

checkCPPOptions :: PackageDescription -> [PackageCheck]
checkCPPOptions pkg =
Expand Down
2 changes: 2 additions & 0 deletions Cabal/Distribution/Simple/BuildTarget.hs
Original file line number Diff line number Diff line change
Expand Up @@ -453,6 +453,7 @@ data ComponentInfo = ComponentInfo {
cinfoAsmFiles:: [FilePath],
cinfoCmmFiles:: [FilePath],
cinfoCFiles :: [FilePath],
cinfoCxxFiles:: [FilePath],
cinfoJsFiles :: [FilePath]
}

Expand All @@ -469,6 +470,7 @@ pkgComponentInfo pkg =
cinfoAsmFiles= asmSources bi,
cinfoCmmFiles= cmmSources bi,
cinfoCFiles = cSources bi,
cinfoCxxFiles= cxxSources bi,
cinfoJsFiles = jsSources bi
}
| c <- pkgComponents pkg
Expand Down
117 changes: 99 additions & 18 deletions Cabal/Distribution/Simple/GHC.hs
Original file line number Diff line number Diff line change
Expand Up @@ -657,14 +657,14 @@ buildOrReplLib forRepl verbosity numJobs pkg_descr lbi lib clbi = do
info verbosity "Building C++ Sources..."
sequence_
[ do let baseCxxOpts = Internal.componentCxxGhcOptions verbosity implInfo
lbi libBi clbi libTargetDir filename
lbi libBi clbi libTargetDir filename
Copy link
Contributor

Choose a reason for hiding this comment

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

Still a spurious whitespace change here

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Don't we want the arguments to be vertically aligned the the function call above?

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah, sure. So long as it's deliberate and not just random reformatting noise.

vanillaCxxOpts = if isGhcDynamic
then baseCxxOpts { ghcOptFPic = toFlag True }
else baseCxxOpts
profCxxOpts = vanillaCxxOpts `mappend` mempty {
ghcOptProfilingMode = toFlag True,
ghcOptObjSuffix = toFlag "p_o"
}
ghcOptProfilingMode = toFlag True,
ghcOptObjSuffix = toFlag "p_o"
}
Copy link
Contributor

Choose a reason for hiding this comment

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

And more whitespace changes here

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Same deliberate, vertical alignment with this whitespace.

sharedCxxOpts = vanillaCxxOpts `mappend` mempty {
ghcOptFPic = toFlag True,
ghcOptDynLinkMode = toFlag GhcDynamicOnly,
Expand Down Expand Up @@ -1083,20 +1083,35 @@ decodeMainIsArg arg
-- 'tail' drops the char satisfying 'pred'
where (r_suf, r_pre) = break pred' (reverse str)

-- | Return C sources, GHC input files and GHC input modules

-- | A collection of:
-- * C input files
-- * C++ input files
-- * GHC input files
-- * GHC input modules
--
-- Used to correctly build and link sources.
data BuildSources = BuildSources {
cSourcesFiles :: [FilePath],
cxxSourceFiles :: [FilePath],
inputSourceFiles :: [FilePath],
inputSourceModules :: [ModuleName]
}

-- | Locate and return the 'BuildSources' required to build and link.
gbuildSources :: Verbosity
-> Version -- ^ specVersion
-> FilePath
-> GBuildMode
-> IO ([FilePath], [FilePath], [ModuleName])
-> IO BuildSources
gbuildSources verbosity specVer tmpDir bm =
case bm of
GBuildExe exe -> exeSources exe
GReplExe exe -> exeSources exe
GBuildFLib flib -> return $ flibSources flib
GReplFLib flib -> return $ flibSources flib
where
exeSources :: Executable -> IO ([FilePath], [FilePath], [ModuleName])
exeSources :: Executable -> IO BuildSources
exeSources exe@Executable{buildInfo = bnfo, modulePath = modPath} = do
main <- findFile (tmpDir : hsSourceDirs bnfo) modPath
let mainModName = fromMaybe ModuleName.main $ exeMainModuleName exe
Expand All @@ -1121,19 +1136,48 @@ gbuildSources verbosity specVer tmpDir bm =
++ display mainModName
++ "' listed in 'other-modules' illegally!"

return (cSources bnfo, [main],
filter (/= mainModName) (exeModules exe))

else return (cSources bnfo, [main], exeModules exe)
else return (main : cSources bnfo, [], exeModules exe)
return BuildSources {
cSourcesFiles = cSources bnfo,
cxxSourceFiles = cxxSources bnfo,
inputSourceFiles = [main],
inputSourceModules = filter (/= mainModName) $ exeModules exe
}

flibSources :: ForeignLib -> ([FilePath], [FilePath], [ModuleName])
else return BuildSources {
cSourcesFiles = cSources bnfo,
cxxSourceFiles = cxxSources bnfo,
inputSourceFiles = [main],
inputSourceModules = exeModules exe
}
else let (csf, cxxsf)
| isCxx main = ( cSources bnfo, main : cxxSources bnfo)
-- if main is not a Haskell source
-- and main is not a C++ source
-- then we assume that it is a C source
| otherwise = (main : cSources bnfo, cxxSources bnfo)

in return BuildSources {
cSourcesFiles = csf,
cxxSourceFiles = cxxsf,
inputSourceFiles = [],
inputSourceModules = exeModules exe
}

flibSources :: ForeignLib -> BuildSources
flibSources flib@ForeignLib{foreignLibBuildInfo = bnfo} =
(cSources bnfo, [], foreignLibModules flib)
BuildSources {
cSourcesFiles = cSources bnfo,
cxxSourceFiles = cxxSources bnfo,
inputSourceFiles = [],
inputSourceModules = foreignLibModules flib
}

isHaskell :: FilePath -> Bool
isHaskell fp = elem (takeExtension fp) [".hs", ".lhs"]

isCxx :: FilePath -> Bool
isCxx fp = elem (takeExtension fp) [".cpp", ".cxx", ".c++"]

-- | Generic build function. See comment for 'GBuildMode'.
gbuild :: Verbosity -> Cabal.Flag (Maybe Int)
-> PackageDescription -> LocalBuildInfo
Expand Down Expand Up @@ -1168,12 +1212,16 @@ gbuild verbosity numJobs pkg_descr lbi bm clbi = do
| otherwise = mempty

rpaths <- getRPaths lbi clbi
(cSrcs, inputFiles, inputModules) <- gbuildSources verbosity
(specVersion pkg_descr) tmpDir bm
buildSources <- gbuildSources verbosity (specVersion pkg_descr) tmpDir bm

let isGhcDynamic = isDynamic comp
let cSrcs = cSourcesFiles buildSources
cxxSrcs = cxxSourceFiles buildSources
inputFiles = inputSourceFiles buildSources
inputModules = inputSourceModules buildSources
isGhcDynamic = isDynamic comp
dynamicTooSupported = supportsDynamicToo comp
cObjs = map (`replaceExtension` objExtension) cSrcs
cxxObjs = map (`replaceExtension` objExtension) cxxSrcs
needDynamic = gbuildNeedDynamic lbi bm
needProfiling = withProfExe lbi

Expand Down Expand Up @@ -1223,7 +1271,7 @@ gbuild verbosity numJobs pkg_descr lbi bm clbi = do
ghcOptLinkFrameworkDirs = toNubListR $
PD.extraFrameworkDirs bnfo,
ghcOptInputFiles = toNubListR
[tmpDir </> x | x <- cObjs]
[tmpDir </> x | x <- cObjs ++ cxxObjs]
}
dynLinkerOpts = mempty {
ghcOptRPaths = rpaths
Expand Down Expand Up @@ -1283,6 +1331,38 @@ gbuild verbosity numJobs pkg_descr lbi bm clbi = do
runGhcProg compileOpts { ghcOptNoLink = toFlag True
, ghcOptNumJobs = numJobs }

-- build any C++ sources
unless (null cxxSrcs) $ do
info verbosity "Building C++ Sources..."
sequence_
[ do let baseCxxOpts = Internal.componentCxxGhcOptions verbosity implInfo
lbi bnfo clbi tmpDir filename
vanillaCxxOpts = if isGhcDynamic
-- Dynamic GHC requires C++ sources to be built
-- with -fPIC for REPL to work. See #2207.
then baseCxxOpts { ghcOptFPic = toFlag True }
else baseCxxOpts
profCxxOpts = vanillaCxxOpts `mappend` mempty {
ghcOptProfilingMode = toFlag True
}
sharedCxxOpts = vanillaCxxOpts `mappend` mempty {
ghcOptFPic = toFlag True,
ghcOptDynLinkMode = toFlag GhcDynamicOnly
}
opts | needProfiling = profCxxOpts
| needDynamic = sharedCxxOpts
| otherwise = vanillaCxxOpts
-- TODO: Placing all Haskell, C, & C++ objects in a single directory
-- Has the potential for file collisions. In general we would
-- consider this a user error. However, we should strive to
-- add a warning if this occurs.
odir = fromFlag (ghcOptObjDir opts)
createDirectoryIfMissingVerbose verbosity True odir
needsRecomp <- checkNeedsRecompilation filename opts
when needsRecomp $
runGhcProg opts
| filename <- cxxSrcs ]

Copy link
Contributor

Choose a reason for hiding this comment

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

This might be personal taste, but is this nicer than for_ cxxSrcs $ \ filename -> ...?

Copy link
Contributor

Choose a reason for hiding this comment

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

Oh, never mind. I see it's patterned on the code below. Best to keep them obviously parallel.

-- build any C sources
unless (null cSrcs) $ do
info verbosity "Building C Sources..."
Expand Down Expand Up @@ -1757,6 +1837,7 @@ installLib verbosity lbi targetDir dynlibTargetDir _builtDir _pkg lib clbi = do

hasLib = not $ null (allLibModules lib clbi)
&& null (cSources (libBuildInfo lib))
&& null (cxxSources (libBuildInfo lib))
has_code = not (componentIsIndefinite clbi)
whenHasCode = when has_code
whenVanilla = when (hasLib && withVanillaLib lbi)
Expand Down
2 changes: 2 additions & 0 deletions Cabal/Distribution/Simple/GHC/IPI642.hs
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@ data InstalledPackageInfo = InstalledPackageInfo {
depends :: [PackageIdentifier],
hugsOptions :: [String],
ccOptions :: [String],
cxxOptions :: [String],
ldOptions :: [String],
frameworkDirs :: [FilePath],
frameworks :: [String],
Expand Down Expand Up @@ -105,6 +106,7 @@ toCurrent ipi@InstalledPackageInfo{} =
Current.depends = map (Current.mkLegacyUnitId . convertPackageId) (depends ipi),
Current.abiDepends = [],
Current.ccOptions = ccOptions ipi,
Current.cxxOptions = cxxOptions ipi,
Current.ldOptions = ldOptions ipi,
Current.frameworkDirs = frameworkDirs ipi,
Current.frameworks = frameworks ipi,
Expand Down
6 changes: 3 additions & 3 deletions Cabal/Distribution/Simple/GHC/Internal.hs
Original file line number Diff line number Diff line change
Expand Up @@ -302,15 +302,15 @@ componentCxxGhcOptions :: Verbosity -> GhcImplInfo -> LocalBuildInfo
-> BuildInfo -> ComponentLocalBuildInfo
-> FilePath -> FilePath
-> GhcOptions
componentCxxGhcOptions verbosity _implInfo lbi bi cxxlbi odir filename =
componentCxxGhcOptions verbosity _implInfo lbi bi clbi odir filename =
mempty {
-- Respect -v0, but don't crank up verbosity on GHC if
-- Cabal verbosity is requested. For that, use --ghc-option=-v instead!
ghcOptVerbosity = toFlag (min verbosity normal),
ghcOptMode = toFlag GhcModeCompile,
ghcOptInputFiles = toNubListR [filename],

ghcOptCppIncludePath = toNubListR $ [autogenComponentModulesDir lbi cxxlbi
ghcOptCppIncludePath = toNubListR $ [autogenComponentModulesDir lbi clbi
,autogenPackageModulesDir lbi
,odir]
-- includes relative to the package
Expand All @@ -320,7 +320,7 @@ componentCxxGhcOptions verbosity _implInfo lbi bi cxxlbi odir filename =
++ [buildDir lbi </> dir | dir <- PD.includeDirs bi],
ghcOptHideAllPackages= toFlag True,
ghcOptPackageDBs = withPackageDB lbi,
ghcOptPackages = toNubListR $ mkGhcOptPackages cxxlbi,
ghcOptPackages = toNubListR $ mkGhcOptPackages clbi,
ghcOptCxxOptions = toNubListR $
(case withOptimization lbi of
NoOptimisation -> []
Expand Down
2 changes: 2 additions & 0 deletions Cabal/Distribution/Simple/JHC.hs
Original file line number Diff line number Diff line change
Expand Up @@ -152,6 +152,8 @@ constructJHCCmdLine lbi bi clbi _odir verbosity =
++ concat [["-i", l] | l <- nub (hsSourceDirs bi)]
++ ["-i", autogenComponentModulesDir lbi clbi]
++ ["-i", autogenPackageModulesDir lbi]
-- Perhaps we need to add the cxxOptions here too?
-- Don't know enough about JHC
++ ["-optc" ++ opt | opt <- PD.ccOptions bi]
-- It would be better if JHC would accept package names with versions,
-- but JHC-0.7.2 doesn't accept this.
Expand Down
Loading