Skip to content

Remove static assert(__traits(compiles, xyz)) from std.conv #3804

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

Conversation

andralex
Copy link
Member

The pattern static assert(__traits(compiles, xyz)) arised probably as a converse to its negation, e.g.:

static assert( __traits(compiles, emplace(&ss2)));
static assert(!__traits(compiles, emplace(&ss2, SS2.init)));

However, the pattern should be avoided for two reasons:

  1. Often it's best to also execute the code and make sure it works as expected (as this PR shows for the octal examples)
  2. When working on the module, a failure of static assert(__compiles) does not show the reason in the error messages. This makes code more difficult to work with gratuitously.

This is just the beginning - could someone please eliminate this pattern from all of Phobos? Thanks!

@@ -4398,7 +4402,7 @@ unittest
@disable this(this);
}
S1 s1 = void;
static assert( __traits(compiles, emplace(&s1, 1)));
emplace(&s1, 1);
Copy link
Member

Choose a reason for hiding this comment

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

assert(s1.i == 1);

@burner
Copy link
Member

burner commented Nov 11, 2015

Other than that LGTM

@burner burner self-assigned this Nov 11, 2015
@@ -4694,7 +4698,7 @@ unittest
}
S1 s = void;
static assert(!__traits(compiles, emplace(&s, 1)));
static assert( __traits(compiles, emplace(&s, &i))); //(works, but deprected)
static assert( __traits(compiles, emplace(&s, &i))); //(works, but deprecated)
Copy link
Member

Choose a reason for hiding this comment

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

Are these used to quell the deprecation message?

Copy link
Member Author

Choose a reason for hiding this comment

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

Correct. If I call it outright the deprecation message shows up.

@andralex
Copy link
Member Author

@burner fixed, and the autotester failure was transitory - thx for the review!

@andralex andralex mentioned this pull request Nov 12, 2015
@andralex
Copy link
Member Author

mmmmmm pass

pliz pliz pullllllllllllllllllll

@JakobOvrum
Copy link
Member

LGTM. Squash and auto-merge? Note that one of these commits snuck its way into your emplace PR, so you should rebase that on master after this is merged.

WalterBright added a commit that referenced this pull request Nov 12, 2015
…mful

Remove `static assert(__traits(compiles, xyz))` from `std.conv`
@WalterBright WalterBright merged commit 74bd803 into dlang:master Nov 12, 2015
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.

4 participants