Skip to content

Make private, remove, or document undocumented public symbols in std.uni #4790

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.

Sign up for GitHub

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
andralex merged 3 commits into from
Oct 1, 2016
Merged

Make private, remove, or document undocumented public symbols in std.uni #4790

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
7bd3b2e
document or make private public symbols
JackStouffer Sep 16, 2016
f5b8e67
Document alias-this work arounds
JackStouffer Sep 16, 2016
727985e
Remove undocumented template hacks
JackStouffer Sep 16, 2016
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
68 changes: 16 additions & 52 deletions std/uni.d
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1898,23 +1898,27 @@ public alias CodepointSet = InversionList!GcPolicy;
*/
public struct CodepointInterval
{
pure:
uint[2] _tuple;
alias _tuple this;
uint[2] tuple;
alias tuple this;

@safe pure nothrow @nogc:

/// Constructor
this(uint low, uint high)
{
_tuple[0] = low;
_tuple[1] = high;
tuple[0] = low;
tuple[1] = high;
}

/// Support for equality testing
bool opEquals(T)(T val) const
Copy link
Contributor Author

@JackStouffer JackStouffer Sep 16, 2016

Choose a reason for hiding this comment

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

Is this opEquals even nessesary? _tuple is the only member of this struct.

Copy link
Member

@jmdavis jmdavis Sep 16, 2016

Choose a reason for hiding this comment

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

If the code is old enough, it might have needed an explicit opEquals to deal with const, but I don't think that that's the case anymore. You should probably verify that though. Another possibility is so that it can be compared with either a tuple or an array.

Copy link
Contributor Author

@JackStouffer JackStouffer Sep 16, 2016

Choose a reason for hiding this comment

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

Another possibility is so that it can be compared with either a tuple or an array.

Yeah, that looks like what it's for

{
return this[0] == val[0] && this[1] == val[1];
}
@property ref inout(uint) a() inout { return _tuple[0]; }
@property ref inout(uint) b() inout { return _tuple[1]; }

/// Access to the interval members
@property ref inout(uint) a() inout { return tuple[0]; }
/// ditto
@property ref inout(uint) b() inout { return tuple[1]; }
}

/**
Expand Down Expand Up @@ -7983,7 +7987,7 @@ version(std_uni_bootstrap)
{
// old version used for bootstrapping of gen_uni.d that generates
// up to date optimal versions of all of isXXX functions
@safe pure nothrow @nogc public bool isWhite(dchar c)
package @safe pure nothrow @nogc bool isWhite(dchar c)
{
import std.ascii : isWhite;
return isWhite(c) ||
Expand Down Expand Up @@ -8324,15 +8328,15 @@ auto asUpperCase(Range)(Range str)
assert("hEllo".asUpperCase.equal("HELLO"));
}

// explicitly undocumented
/// ditto
auto asLowerCase(Range)(auto ref Range str)
Copy link
Contributor Author

@JackStouffer JackStouffer Sep 16, 2016

Choose a reason for hiding this comment

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

@MartinNowak git says that you made these overloads. Why didn't you document them? I don't see why this should be hidden from the user.

Copy link
Member

@jmdavis jmdavis Sep 16, 2016

Choose a reason for hiding this comment

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

It's not exactly pretty have all of those overloads in the docs. If it weren't for the fact that the secondary overloads use auto ref, I'd argue that it should all just be merged into one function that used static if, but I don't think that we should normally be putting auto ref on ranges like that. So, maybe it should be documented, but it is kind of ugly to have a bunch of overloads like that which basically only differ by their template constraints.

Copy link
Contributor Author

@JackStouffer JackStouffer Sep 16, 2016

Choose a reason for hiding this comment

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

IMO the ddoc looks much better then the ddox version on functions with multiple overloads

if (isConvertibleToString!Range)
{
import std.traits : StringTypeOf;
return asLowerCase!(StringTypeOf!Range)(str);
}

// explicitly undocumented
/// ditto
auto asUpperCase(Range)(auto ref Range str)
if (isConvertibleToString!Range)
{
Expand Down Expand Up @@ -8521,6 +8525,7 @@ auto asCapitalized(Range)(Range str)
assert("hEllo".asCapitalized.equal("Hello"));
}

/// ditto
auto asCapitalized(Range)(auto ref Range str)
if (isConvertibleToString!Range)
{
Expand Down Expand Up @@ -8843,16 +8848,6 @@ void toLowerInPlace(C)(ref C[] s) @trusted pure
{
toCaseInPlace!(LowerTriple)(s);
}
// overloads for the most common cases to reduce compile time
@safe pure /*TODO nothrow*/
{
void toLowerInPlace(ref char[] s)
{ toLowerInPlace!char(s); }
void toLowerInPlace(ref wchar[] s)
{ toLowerInPlace!wchar(s); }
void toLowerInPlace(ref dchar[] s)
{ toLowerInPlace!dchar(s); }
}

/++
Converts $(D s) to uppercase (by performing Unicode uppercase mapping) in place.
Expand All @@ -8865,16 +8860,6 @@ void toUpperInPlace(C)(ref C[] s) @trusted pure
{
toCaseInPlace!(UpperTriple)(s);
}
// overloads for the most common cases to reduce compile time/code size
@safe pure /*TODO nothrow*/
{
void toUpperInPlace(ref char[] s)
{ toUpperInPlace!char(s); }
void toUpperInPlace(ref wchar[] s)
{ toUpperInPlace!wchar(s); }
void toUpperInPlace(ref dchar[] s)
{ toUpperInPlace!dchar(s); }
}

/++
If $(D c) is a Unicode uppercase $(CHARACTER), then its lowercase equivalent
Expand Down Expand Up @@ -8914,17 +8899,6 @@ S toLower(S)(S s) @trusted pure
static import std.ascii;
return toCase!(LowerTriple, std.ascii.toLower)(s);
}
// overloads for the most common cases to reduce compile time
@safe pure /*TODO nothrow*/
{
string toLower(string s)
{ return toLower!string(s); }
wstring toLower(wstring s)
{ return toLower!wstring(s); }
dstring toLower(dstring s)
{ return toLower!dstring(s); }
}


@system unittest //@@@BUG std.format is not @safe
{
Expand Down Expand Up @@ -9080,16 +9054,6 @@ S toUpper(S)(S s) @trusted pure
static import std.ascii;
return toCase!(UpperTriple, std.ascii.toUpper)(s);
}
// overloads for the most common cases to reduce compile time
@safe pure /*TODO nothrow*/
{
string toUpper(string s)
{ return toUpper!string(s); }
wstring toUpper(wstring s)
{ return toUpper!wstring(s); }
dstring toUpper(dstring s)
{ return toUpper!dstring(s); }
}

@safe unittest
{
Expand Down