Adds -ghc-version flag to ghc.
ClosedPublic

Authored by angerman on Oct 29 2017, 1:26 AM.

Details

Summary

When building the rts with ghc (e.g. using ghc as a c compiler), ghc's
"Value Add"[1] is, it includes adding -include /path/to/ghcversion.h. For
this it looksup the rts package in the package database, which--if
empty--fails. Thus to allow compiling C files with GHC, we add the
-ghc-version flag, which takes the path to the ghcversion.h file.

A -no-ghc-version flag was omitted, as at that point it becomes
questionable why one would use ghc to compile c if one doesn't
any of the added value.

[1] from compiler/main/DriverPipeline.hs

  • add package include paths even if we're just compiling .c
  • files; this is the Value Add(TM) that using ghc instead of
  • gcc gives you :)

Diff Detail

Repository
rGHC Glasgow Haskell Compiler
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
angerman created this revision.Oct 29 2017, 1:26 AM
bgamari requested changes to this revision.Oct 29 2017, 7:33 PM

I'll admit I do wish that we were more consistent in our use of gnu option convention (e.g. --ghc-version). That being said, I'm not sure we can do much about it now so fair enough.

However, this surely needs documentation, even if only in debugging.rst.

This revision now requires changes to proceed.Oct 29 2017, 7:33 PM

I'll admit I do wish that we were more consistent in our use of gnu option convention (e.g. --ghc-version). That being said, I'm not sure we can do much about it now so fair enough.

I think it's far too late for that, as ghc's primary convention is -- indicates primary operating modes (e.g. --interactive, --make, etc.) This isn't a primary operating mode, so single - is correct.

In some dystopian future, we'll have --2, and that will put ghc into optparse-applicative mode :)

... alright, I'll show myself out.

I'll admit I do wish that we were more consistent in our use of gnu option convention (e.g. --ghc-version). That being said, I'm not sure we can do much about it now so fair enough.

I think it's far too late for that, as ghc's primary convention is -- indicates primary operating modes (e.g. --interactive, --make, etc.) This isn't a primary operating mode, so single - is correct.

Ahhh, I had never noticed that; thanks for point this out. Yes, I suppose that is fairly reasonable.

Just for the record, this is my current ghcversion.h

#ifndef __GHCVERSION_H__
#define __GHCVERSION_H__

#define __GLASGOW_HASKELL__ 803

#define __GLASGOW_HASKELL_PATCHLEVEL1__ 20171028

#define MIN_VERSION_GLASGOW_HASKELL(ma,mi,pl1,pl2) (\
   ((ma)*100+(mi)) <  __GLASGOW_HASKELL__ || \
   ((ma)*100+(mi)) == __GLASGOW_HASKELL__    \
          && (pl1) <  __GLASGOW_HASKELL_PATCHLEVEL1__ || \
   ((ma)*100+(mi)) == __GLASGOW_HASKELL__    \
          && (pl1) == __GLASGOW_HASKELL_PATCHLEVEL1__ \
          && (pl2) <= __GLASGOW_HASKELL_PATCHLEVEL2__ )

#endif /* __GHCVERSION_H__ */

and is passed as --include.

Now I'm not absolutely sure why this lives in the rts and not in the compiler package, (or even in $topdir). However, as ghc surely knows these values, couldn't it just generate this file on the fly.
I'm not sure that the c compilers we intend to support support "complex" macros via -D.

austin resigned from this revision.Nov 9 2017, 5:43 PM
angerman updated this revision to Diff 14614.Nov 9 2017, 8:03 PM
  • Add DynFlags signature.
bgamari requested changes to this revision.Nov 10 2017, 9:02 PM

This still needs documentation.

This revision now requires changes to proceed.Nov 10 2017, 9:02 PM
angerman edited the summary of this revision. (Show Details)Nov 11 2017, 12:27 AM
angerman updated this revision to Diff 14623.Nov 11 2017, 12:43 AM
angerman edited the summary of this revision. (Show Details)
  • Adds docs
angerman updated this revision to Diff 14710.Nov 16 2017, 9:03 AM
  • rebase
  • Adds type
bgamari updated this revision to Diff 14714.Nov 16 2017, 6:00 PM

Fix documentation

Guess, this is good to go then?

geekosaur added inline comments.Nov 17 2017, 2:39 AM
docs/users_guide/using.rst
1085

Either this should be '... to be included, this ...', or it should be rewritten as something like

In some cases, the compiler's package database does not contain the `rts`

or one wants to specify a specific ``ghcversions.h`` to be included.
geekosaur added inline comments.Nov 17 2017, 2:46 AM
docs/users_guide/using.rst
1085

ugh, that got mangled.

"In some cases, the compiler's package database does not contain the rts package, or one wants to specify a specific ghcversions.h to be included."

angerman updated this revision to Diff 14719.Nov 17 2017, 5:30 AM
  • cleanup.
angerman marked 2 inline comments as done.Nov 17 2017, 5:34 AM

Ok. Now that @geekosaur's remarks are integrated as well, can we merge this?

bgamari accepted this revision.Nov 17 2017, 10:10 AM

Yes, looks good to me.

This revision is now accepted and ready to land.Nov 17 2017, 10:10 AM
This revision was automatically updated to reflect the committed changes.