Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add post-switches to config file sections #2281

Merged
merged 3 commits into from Aug 27, 2017

Conversation

kinke
Copy link
Member

@kinke kinke commented Aug 20, 2017

For switches to be appended after the user switches (or right before the first -run switch).

The sections inherit it (as well as switches) from the default section if it isn't overridden explicitly.

Fixes issue #2186.

@kinke kinke force-pushed the config branch 2 times, most recently from 7d039e2 to 984bb93 Compare August 20, 2017 12:20
For switches to be appended after the user switches (or right before the
first `-run` switch).

The sections inherit it (as well as `switches`) from the `default` section
if it isn't overridden explicitly.

Fixes issue ldc-developers#2186.
@JohanEngelen
Copy link
Member

It'd be very nice to get this into 1.4. Without it, it is a lot more work to use LTO/sanitizer builds of the standard library.
Biggest complaint is that tests are missing.

@kinke
Copy link
Member Author

kinke commented Aug 24, 2017

Sure thing, but as there was no feedback, I didn't want to postpone beta1 further.

@kinke
Copy link
Member Author

kinke commented Aug 24, 2017

Oh and wrt. the tests - as all config files use it, it's tested everywhere, at least that both sets of switches are used (and that the post-switches are inserted before -run). I don't think we output the full command line anywhere yet...

@JohanEngelen
Copy link
Member

I'm working on the tests.

@JohanEngelen
Copy link
Member

(pfiew, ok, the git pushing worked)

@JohanEngelen JohanEngelen added this to the 1.4.0 milestone Aug 24, 2017
// CHECK-SAME: -post-two-switch


// RUN: not %ldc -I=%runtimedir/src -conf=%S/inputs/post_switches.conf -v -L-user-passed-switch -run %s -L-after-run | FileCheck %s --check-prefix=WITHrUN
Copy link
Member

Choose a reason for hiding this comment

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

the WITHrUN is not a typo btw. Lit confuses WITHRUN: lines for RUN: lines.

@JohanEngelen
Copy link
Member

Is this good to merge?

// append 'post-switches', but before a first potential '-run'
size_t runIndex = 0;
for (size_t i = 1; i < args.size(); ++i) {
if (strcmp(args[i], "-run") == 0 || strcmp(args[i], "--run") == 0) {
Copy link
Member

Choose a reason for hiding this comment

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

In an ideal world, this would be args[i] == "-runs"s || args[i] == "--run"s. (Only made it into C++14, though, I think.)

@dnadlinger dnadlinger merged commit bd4bf96 into ldc-developers:master Aug 27, 2017
@kinke kinke deleted the config branch August 27, 2017 18:38
@kinke kinke mentioned this pull request Aug 27, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants