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

Issue 6288 - std.conv.to removes const/immutable when converting a class #271

Merged
merged 6 commits into from Sep 22, 2011

Conversation

9rnsr
Copy link
Contributor

@9rnsr 9rnsr commented Sep 20, 2011

@schveiguy
Copy link
Member

Wow, nice exhaustive unit tests, Jonathan would be proud :) It took me a while to figure out what's going on in there. You may want to add a comment, it's quite confusing, yet cool.

The other changes look good. Assuming this passes unit tests, it can be merged IMO.

static if (n == 1) alias toConst Mod2Conv;
static if (n == 2) alias toShared Mod2Conv;
static if (n == 3) alias toSharedConst Mod2Conv;
static if (n == 4) alias toImmutable Mod2Conv;
Copy link
Member

Choose a reason for hiding this comment

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

It's not a big deal, but I'd use else static if instead of using static if for each branch. It's more logically correct in the sense that it's not possible for more than one branch to be correct and the else static if reflects that whereas use static if for all of them does not, but it likely speeds up compilation as well (obviously not by much, but every bit adds up), since it doesn't have to evaluate the secondary branches.

@jmdavis
Copy link
Member

jmdavis commented Sep 22, 2011

LOL. Yeah. Gotta love the thoroughness of those unit tests. Maybe I've been writing too many unit tests like that though, since I don't really find them particularly confusing (though they are the sort of tests that you have to read carefully).

@9rnsr
Copy link
Contributor Author

9rnsr commented Sep 22, 2011

@schveiguy
Indeed, this test was a little hard to read.
I added comments and improved the template names.

@jmdavis
OK. Use else static if instead of static if.

schveiguy added a commit that referenced this pull request Sep 22, 2011
Issue 6288 - std.conv.to removes const/immutable when converting a class
@schveiguy schveiguy merged commit 8b6ef3b into dlang:master Sep 22, 2011
@schveiguy
Copy link
Member

Gah, I just merged it. But there is no change to changelog.dd, can you do that Hari? Just do it directly, no need to post pull request.

@9rnsr
Copy link
Contributor Author

9rnsr commented Sep 22, 2011

Pushed it. Thanks.
2082d9f

marler8997 pushed a commit to marler8997/phobos that referenced this pull request Nov 10, 2019
Use "-i" to prevent rdmd from having to invoke compiler twice.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants