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

std.getopt: Make private functions safe. #5351

Merged
merged 2 commits into from
Apr 26, 2017

Conversation

jondegenhardt
Copy link
Contributor

This change make a pair of internal std.getopt functions @safe. This enables some additional @safe getopt use: callbacks via delegates and config option setting. @safe unit tests for both are included.

I also went through all the existing unit tests to see if any could be converted from @system to @safe. However, the existing @system unit tests take the address of local variables are cannot be @safe.

This PR is a follow-up to PR #5347.

@JackStouffer
Copy link
Member

@nogc pure nothrow should be added as well if applicable

Copy link
Member

@quickfur quickfur left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!

@quickfur
Copy link
Member

And yes, if pure, nothrow, etc., are applicable, they should be added as well (to unittests and non-template functions only, of course).

@jondegenhardt
Copy link
Contributor Author

Added pure nothrow @nogc to setConfig. Also changed it to use final switch. However, optMatch calls functions not yet marked, so not none of these attributes could be applied to it. Also means none of the unit tests can be marked with these attributes.

@dlang-bot dlang-bot merged commit cc55bb2 into dlang:master Apr 26, 2017
@jondegenhardt jondegenhardt deleted the getopt-safe-attribute branch April 26, 2017 18:52
@CyberShadow
Copy link
Member

This pull request introduced a regerssion:
https://issues.dlang.org/show_bug.cgi?id=17650

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.

6 participants