Rsp support #32
Rsp support #32
Conversation
| if(!options.keepDlangFiles) { | ||
| foreach(fileName; options.dFileNames) | ||
| remove(fileName); | ||
| } | ||
| enforce(status == 0, "Executing `" ~ args.join(" ") ~ "` failed with exit code\n" ~ status.text); |
There was a problem hiding this comment.
moved this to the end so files get cleaned up even if things fail (user can obviously specify to keep the files if they want to inspect them)
| filter!(a => a.extension == ".dpp" || a.extension == ".d") | ||
| .front | ||
| .stripExtension | ||
| ~ exeExtension; |
There was a problem hiding this comment.
There's no need to handle this here, if we don't mess with file ordering then we get dmd's behaviour for free.
| ~ exeExtension; | ||
| dlangCompilerArgs = args[1..$] | ||
| .map!(a => a.extension == ".dpp" ? toDFileName(a) : a) | ||
| .array; |
There was a problem hiding this comment.
avoiding re-ordering files on the command line.
| // ported from dmd's function of the same name. | ||
| // only modifications are to use builtin arrays | ||
| // and phobos in place of dmd's bespoke types | ||
| bool response_expand(ref const(char)*[] args) |
There was a problem hiding this comment.
I was gonna port some dmd tests for this function as well, but..... I couldn't find them. I suspect they don't exist
|
|
||
| foreach(dppFileName; options.dppFileNames) | ||
| preprocess!File(options, dppFileName, options.toDFileName(dppFileName)); | ||
|
|
||
| if(options.preprocessOnly) return; | ||
|
|
||
| const args = options.dlangCompiler ~ options.dlangCompilerArgs; | ||
| const res = execute(args); | ||
| enforce(res.status == 0, "Could not execute `" ~ args.join(" ") ~ "`:\n" ~ res.output); | ||
| const status = spawnProcess(args).wait(); |
There was a problem hiding this comment.
Why spawnProcess.wait instad of execute?
There was a problem hiding this comment.
So that the compiler output gets out regardless of what d++ thinks. pragma(msg,...) needs to get out even on success, as do warnings. Also, you want to see whatever output you might have got before (or during) an infinite loop in the compiler or ctfe, so we can't wait for the compiler to finish before printing, might as well let it do its own printing to std{out/err}.
|
|
||
| foreach(dppFileName; options.dppFileNames) | ||
| preprocess!File(options, dppFileName, options.toDFileName(dppFileName)); | ||
|
|
||
| if(options.preprocessOnly) return; | ||
|
|
||
| const args = options.dlangCompiler ~ options.dlangCompilerArgs; | ||
| const res = execute(args); | ||
| enforce(res.status == 0, "Could not execute `" ~ args.join(" ") ~ "`:\n" ~ res.output); |
There was a problem hiding this comment.
no need to have this, if the target compiler can handle what's been given then we just pass it through.
There was a problem hiding this comment.
It's needed or else the compiler failing would cause d++ to return 0.
There was a problem hiding this comment.
sorry, I commented in the wrong place. This line is still there, just moved down a bit.
|
Merging this without a test gives me the heebie jeebies, especially when the codecov check says -13% coverage. You said that this is needed to work with your dub work - I assume that means you tested it manually? How so? |
| dppFileNames = args.filter!(a => a.extension == ".dpp").array; | ||
| enforce(dppFileNames.length != 0, "No .dpp input file specified\n" ~ usage); |
There was a problem hiding this comment.
This is where I meant to write this:
no need to have this, if the target compiler can handle what's been given then we just pass it through.
|
Things that should be tested:
|
For compatibilities sake I just took dmd's implementation and did the bare minimum to make it work.
There aren't any tests so far, I haven't properly navigated the test setup you're using here yet.
This is all that's necessary to work with https://github.com/John-Colvin/dub/tree/dpp_support