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
Details
- Reviewers
mpickering thomie ezyang bgamari - Commits
- rGHCebcbfba7bbf0: Introduce flag -keep-hscpp-files
- Trac Issues
- #10869
Testcase T10869
Diff Detail
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.
Comment Actions
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!
Comment Actions
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". | |
Comment Actions
- Docu: Add suggestions of bgamari. (Trac #10869)
- Tests: Add suggestions of linter. (Trac #10869)
Comment Actions
I added the requested changes by bgamari and by the linter. Please feel free to request additional changes!
Comment Actions
@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!
Comment Actions
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.
Comment Actions
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
Comment Actions
@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. | |
Comment Actions
@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!
Comment Actions
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.
Comment Actions
- 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.
Comment Actions
LGTM
I guess specifying -E together with -keep-hscpp-files will write both files? That's ok though.
Comment Actions
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.
Comment Actions
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