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

Improve std.conv.to (1) #118

Merged
merged 7 commits into from
Jul 13, 2011
Merged

Improve std.conv.to (1) #118

merged 7 commits into from
Jul 13, 2011

Conversation

9rnsr
Copy link
Contributor

@9rnsr 9rnsr commented Jun 23, 2011

The main commit is d7aac207, and others are code formatting, bug fix, and so on.

Changes:

  1. Remove conversion feature using member template function to!T() with user type.
    2-1. Support conversion feature using opCast!T() with aggregate types.
    2-2. Support conversion feature using constructor like T(s) with struct types.

{
return to!T(cast(int) value);
}

/// Unsigned integers (uint and ulong) to string.
T toImpl(T, S)(S input)
if (staticIndexOf!(Unqual!S, uint, ulong) >= 0 && isSomeString!T)
if (staticIndexOf!(Unqual!S, uint, ulong) >= 0 &&
isSomeString!T)
{
Copy link
Member

Choose a reason for hiding this comment

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

I didn't even realize that staticIndexOf existed. Looks like it could definitely be a useful function.

@jmdavis
Copy link
Member

jmdavis commented Jul 1, 2011

Actually, there is one change that needs to be made before this can be merged in. The "Remove conversion feature using member template function to!T() with user type" part is going too far. Adding the opCast functionality is fine, and I think that getting rid of the to member function is fine in the long run, but the current changes will break code. The old toImpl which deals with the to member function should be put back but have its documentation altered to indicate that it has been scheduled for deprecation and have a pragma inside of it which indicates that it has been scheduled for deprecation, with both messages indicating that the type should an overloaded opCast instead. You'll probably have to add a constraint to the old template to indicate that it shouldn't be used if opCast has been overloaded for that type, but beyond that, it shouldn't be all that hard to put it back in. And we should put it back in to avoid breaking code. You can look at std.file to see examples of how to deal with deprecation messages, and at this point, the deprecation message should indicate that that version of toImpl will be deprecated in January 2012, but I can adjust the deprecation message later with the appropriate dates. The key thing is that the old version of toImpl which uses a member function called to should not be immediately removed, since doing so will break code. It needs to be scheduled for deprecation, then deprecated, and then finally removed.

9rnsr added 7 commits July 7, 2011 08:18
…straint of S -> constraint of T) for code readability.
…ible

The two templates has same purpose, but bit different behaviors.
implicitlyConverts(S, T) has following semantics:
   is(S : T) and allow initializer conversion (T t = s; -> T t = T(s);)
   Including initializer conversion is inconvenience for my to!() improvement in after commits, so replace it.
1. Remove conversion feature using member template function to!T() of user type.
2-1. Support conversion feature using opCast!T() with aggregate types.
2-2. Support conversion feature using constructor like T(s) with struct types.
@9rnsr
Copy link
Contributor Author

9rnsr commented Jul 6, 2011

Rebase, revert removing feature that using method to, and add deprecation message.

jmdavis added a commit that referenced this pull request Jul 13, 2011
@jmdavis jmdavis merged commit 184110a into dlang:master Jul 13, 2011
kuettler pushed a commit to kuettler/phobos that referenced this pull request Feb 6, 2018
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.

2 participants