-
-
Notifications
You must be signed in to change notification settings - Fork 705
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.typecons.Typedef has opCmp when base type does not #10382
Labels
Comments
simen.kjaras commented on 2019-08-09T07:14:08ZThis would perhaps more correctly be filed as 'int does not have opCmp'. Since int does support the <, <=, >=, > operators it kinda, sorta has opCmp, but no member by that name. That's not really an option for Typedef, as it's not a magic type like int. At the same time, we absolutely expect MyInt.init < MyInt.init to compile.
There may be a good argument for adding opCmp and friends to built-in types to make generic code more generic, but removing them from Typedef is no solution. |
simen.kjaras commented on 2019-08-09T07:51:39ZAlternatively, Typedef should contain only a single member and use aliases for the rest. Something like this:
struct Typedef(T, T init = T.init, string cookie=null) {
// Renaming the current Typedef:
TypedefImpl!(T, init, cookie) _payload;
alias _payload this;
static foreach (e; __traits(allMembers, T))
mixin("alias "~e~" = _payload."~e~";");
} |
atila.neves (@atilaneves) commented on 2019-08-09T08:21:20ZBut `int` *doesn't* have `opCmp`. I found this by trying to wrap a generic type for Python and implementing the comparison. std.typecons.Typedef completely broke things and I have to special-case for it, because `hasMember!(Typedef, "opCmp")` returns true, but then one can't actually forward to it because it doesn't actually exist.
Incidentally, the reason the bug happens is because `Typedef` uses `Proxy` in its implementation, and `Proxy` defines `opCmp`. |
simen.kjaras commented on 2019-08-09T11:37:22ZBut `int` *is* comparable using <, <=, >=, and >, so any (non-built-in) type pretending to be an int needs to have `opCmp`. What you're asking for is basically to cripple `Typedef` because you need to write some workaround code. Alternatively, you're asking for what I said in comment 2, but have elected to ignore that.
Actually, even with my `alias this` solution in comment 2, you get two members: `_payload`, and `opAssign`. The former we've defined, the latter the compiler makes for us. `int` does not have `opAssign`, so we can't forward to that, thus again causing possible issues. In fact, `int` doesn't have any members at all, if you ask `__traits(allMembers)`. Thus, any member whatsoever in `Typedef!int` could be problematic in generic code. I hope we can agree that `Typedef!int` needs to have at least one member.
Actually, we can get rid of all fields by (ab)using `align` and some creative casting:
template Typedef(T) {
align ((T.sizeof + 1) & -2) // powers of 2 only
struct Typedef {
ref auto _payload() {
import std.typecons : TypedefImpl = Typedef;
return *cast(TypedefImpl!T*)&this;
}
alias _payload this;
// alias all members of the wrapped type
static if (__traits(compiles, __traits(allMembers, T)))
static foreach (e; __traits(allMembers, T))
mixin("alias _payload."~e~" "~e~";");
}
}
This still leaves us with a single method (`_payload()`) that I haven't found a way to get rid of yet. Also, it's hella ugly. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
atila.neves (@atilaneves) reported this on 2019-08-08T16:51:54Z
Transfered from https://issues.dlang.org/show_bug.cgi?id=20117
CC List
Description
The text was updated successfully, but these errors were encountered: