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

Fix issue 19738 -- std.range.choose assignment breaks @safe-ty #7348

Merged
merged 1 commit into from
Jan 10, 2020

Conversation

schveiguy
Copy link
Member

This was more tricky than I originally thought, mostly due to the existing requirement that chooseAmong works with strings during CTFE.

Essentially, we only trust the fetching of the pointer to the chosen range, everything else should be inferred.

I'm not sure why I had to remove return scope, but that doesn't seem to work, I let the attributes get inferred. The error messages are totally unrelated to this code, it's just not correctly inferring safety with those attributes. But I don't really understand what the return scope did in the first place.

Should obsolete #6933

@thewilsonator @dnadlinger

@dlang-bot
Copy link
Contributor

Thanks for your pull request, @schveiguy!

Bugzilla references

Auto-close Bugzilla Severity Description
19738 regression std.range.choose assignment breaks @safe-ty

Testing this PR locally

If you don't have a local development environment setup, you can use Digger to test this PR:

dub fetch digger
dub run digger -- build "stable + phobos#7348"

@thewilsonator
Copy link
Contributor

Doc tester timed out please force push to restart.

@schveiguy
Copy link
Member Author

I don't have much confidence that the macair is going to succeed on a third run. So I'll force push. Meanwhile, @braddr you have any ideas? It seems to time out quite a bit.

…in case the

individual ranges have system opAssign.
@dlang-bot dlang-bot merged commit e2e7159 into dlang:stable Jan 10, 2020
@schveiguy schveiguy deleted the choosesafeassign branch January 10, 2020 05:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants