Keep the output of the CPP preprocessor phase as .hscpp files. A .hscpp file is only created, if a module gets compiled and uses the preprocessor.
Details
- Reviewers
mpickering thomie ezyang bgamari - Commits
- rGHCebcbfba7bbf0: Introduce flag -keep-hscpp-files
- Trac Issues
- #10869
Testcase T10869
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.
This is my first contribution to GHC, where I wrote real Haskell code. Therefore this is just a draft version and it needs some additional improvments.
1: CPP processing. See: ./compiler/main/DriverPipeline.hs.runPhase (RealPhase (Cpp sf)) (ca line 940):
When the -keep-hspp-file flag is set, the output file of the CPP preprocessor is copied to the *.hspp file.
2: END processing See: ./compiler/main/DriverPipeline.hs.pipeLoop (ca line 680):
The output of the last compiler phase is copied. With the -E option, GHC processing stops early and copies the output of the CPP phase to the *.hspp file.
If the user specifies both flags -E and -keep-hspp-file, then the file will be copied a first time in my new code during CPP processing and a second time here.
This is bad! Therefore I added code to avoid duplicate copying.
However, with the -E flag set, it would be better to skip the copying during CPP processing.
Unfortunately I was unable to check the -E flag in the CPP processing code! The "Mode Flags" like the -E flag, only exist in ./ghc/Main.hs.
I didn't find anything like gopt for the mode flags!! How can I check them??
I looked at the 'stop_phase' field in the PipeEnv Structure, but with or without the -E flag, it has the same value (Hsc).
Please advice! Many thanks!
Thanks for your contribution @RolandSenn!
The patch itself looks great. However, do you suppose you could amend the differential summary to describe what the patch is doing?
docs/users_guide/separate_compilation.rst | ||
---|---|---|
415 | Can you wrap the .hspp in quotes? Also, replace "precompiler" with "preprocessor". |
- Docu: Add suggestions of bgamari. (Trac #10869)
- Tests: Add suggestions of linter. (Trac #10869)
I added the requested changes by bgamari and by the linter. Please feel free to request additional changes!
@bgamari : You wrote:
However, do you suppose you could amend the differential summary to describe what the patch is doing?
What do you exactly mean by this? Should I change/improve the description in docs/users_guide/separate_compilation.rst? Or do you simply mean the two little changes (quotes and preprocessor)? Please advice! Many thanks!
The differential summary (the bit shown under the "Summary" heading of this page) will become the commit message when this patch is landed. I was simply suggesting that you add a bit of text describing the goal of the patch for posterity.
Hello Ben,
I'll do this. But this week I'm away from my desktop computer. So maybe
it will take till mid next week.
Thanks and best wishes. Roland
@RolandSenn: sorry for the late reply.
The phases go like this:
- Unlit (.lhs -> .hs)
- Cpp (.hs -> .hscpp)
- HsPp (.hscpp -> .hspp)
- Hsc (.hspp -> ...)
So it is the HsPp phase (https://downloads.haskell.org/~ghc/8.4.3/docs/html/users_guide/phases.html?highlight=hspp#options-affecting-a-haskell-pre-processor) that outputs .hspp, not the Cpp phase, as your patch seems to imply.
It is probably better to compare -keep-hspp-files with the likes of -keep-hc/s/llvm-files, instead of with -E. -E is special, in that it stops compilation early:
, defFlag "E" (PassFlag (setMode (stopBeforeMode anyHsc)))
compiler/main/DriverPipeline.hs | ||
---|---|---|
767 | I think adding keep_hspp = gopt Opt_KeepHsppFiles dflags here could make your patch much shorter. |
@thomie: Many thanks for your code review and your kind hints to improve the patch.
Your table about the phases and the corresponding output file names suggests, that we should have 2 new flags:
-keep-hscpp-file: it will keep the output file of the C pre-processor and name it *.hscpp
-keep-hspp-file: it will keep the output file of the Haskell pre-processor (specified with the -F and -pgmF flags) and name it *.hspp.
The file produced by the -E flag however, will continue to be named *.hspp, although it is the output of the C pre-processor. So I will NOT change the behaviour of the -E flag!
The file produced by the -E flag however, will continue to be named *.hspp, although it is the output of the C pre-processor.
Not quite.
-E stops after the HsPp phase, and produces a *.hspp file.
Phases always follow upon each other.
Unlit -> CPP -> HsPp
So the file that -E produces has undergone all 3 of Unlit and Cpp and HsPp, in that order.
See also the function runPhase (+ docstring).
we should have 2 new flags
Two new flags seems excessive, unless someone is asking for it.
- Improve patch after code review (Trac #10869).
- Rename flag form keep-hspp-files to kepp-hscpp-files.
- Do not copy the file but reuse code from the keep-hc/s/llvm-files flags.
LGTM
I guess specifying -E together with -keep-hscpp-files will write both files? That's ok though.
I guess specifying -E together with -keep-hscpp-files will write both files?
Yes! There exists a use case for this: If you additionally specifiy the -F and -pgmF=XYZ flags, then you see in the *.hscpp file the input and in the *.hspp file the output of your XYZ pre-processor.
RolandSenn: one last thing, before @bgamari gets here. Maybe you could add a blurp to the release notes about this new flag.
docs/users_guide/8.8.1-notes.rst