Skip to content

Port stdlib/public/stubs/GlobalObjects to Windows #6239

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

Merged
merged 1 commit into from
Jan 9, 2017
Merged

Port stdlib/public/stubs/GlobalObjects to Windows #6239

merged 1 commit into from
Jan 9, 2017

Conversation

hughbe
Copy link
Contributor

@hughbe hughbe commented Dec 12, 2016

This avoids an assertion/crash compiling swift code once the stdlib and compiler has been built.

@compnerd
Copy link
Member

Can we change this to use C++11's <random> interfaces instead? I feel like that is more standards compliant and doesnt require us to consider seeding issues and can give us the same number space to pull from.

@compnerd
Copy link
Member

@jckarter and @jrose-apple, I think you guys should chime in here as well.

@hughbe
Copy link
Contributor Author

hughbe commented Dec 13, 2016

Something like this could work, and we could get rid of all the platform specific code:

  std::random_device rd;
  std::mt19937_64 mt(rd());
  std::uniform_int_distribution<uint64_t> dist;

  while (true)
  {
    std::cout << dist(mt) << "\n";
  }

@jrose-apple
Copy link
Contributor

Seems reasonable to me, but this bit of code is very much in service of the stdlib (to change hashing behavior between runs). @dabrahams?

@jrose-apple
Copy link
Contributor

We'd also be fine with using external libraries on Windows if we had to, as long as they're installed on systems by default. We link to Foundation on Apple platforms!

@compnerd
Copy link
Member

Thanks @hughbe, this looks much better!

@compnerd
Copy link
Member

@swift-ci please test and merge

@jrose-apple
Copy link
Contributor

Um, I'd definitely still like Dave to take a look. This happens as part of a static constructor.

@compnerd
Copy link
Member

@jrose-apple, yeah, I was thinking about that after wards. I think that we should also benchmark this to make sure it doesn't giant the startup too much.

@compnerd
Copy link
Member

@swift-ci please test

@swift-ci
Copy link
Contributor

Build failed
Jenkins build - Swift Test OS X Platform
Git Commit - 427aa901ab9fa35d4fbca4c8a26b68753dbf868d
Test requested by - @compnerd

@swift-ci
Copy link
Contributor

Build failed
Jenkins build - Swift Test Linux Platform
Git Commit - 427aa901ab9fa35d4fbca4c8a26b68753dbf868d
Test requested by - @compnerd

@compnerd
Copy link
Member

CC: @dabrahams mind taking a look? this is one of the few pieces that prevents building on Windows.

@dabrahams
Copy link
Contributor

I'm going to pass this over to @airspeedswift because I know he's interested in creating a public random number facility around this basic ideas.

@compnerd
Copy link
Member

@airspeedswift think you can get to this soon? This is the last piece needed to repair the windows builds!

@hughbe
Copy link
Contributor Author

hughbe commented Jan 9, 2017

@airspeedswift friendly ping!

@dabrahams
Copy link
Contributor

LGTM; merging

@dabrahams dabrahams merged commit 9392ac5 into swiftlang:master Jan 9, 2017
@hughbe hughbe deleted the stubs-rand branch January 20, 2017 10:21
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.

5 participants