-
-
Notifications
You must be signed in to change notification settings - Fork 706
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
Fix Issue 16108: to!string fails on struct with disabled postblit #5425
Conversation
|
| if (isAggregateType!S && !isCopyable!S) | ||
| { | ||
| return toImpl!T(arg); | ||
| } | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can these not be merged into a single
T to(A...)(auto ref A args)
i.e. amending the very first overload and removing the "fix issue" overloads?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No because then you would be modifying the behavior of the function for several types, most notably ranges.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't follow. Right now we have overloads in to and overloads in toImpl, with "fix issue" to overloads having the same constraints as their toImpl counterparts. Can we not have just the toImpl overloads? What am I missing?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, sorry I didn't see your proposed template signature and misunderstood.
Well, you definitely never want to pass a static array by value, as that would cause a lot of the code to return references to stack data. That's why the original special case was added.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh, I see now, this prevents matching against other toImpl overloads. Thanks.
std/conv.d
Outdated
| auto a = A(); | ||
| assert(to!string(a) == "0:false"); | ||
| } | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How about adding a test without a custom toString?
@safe pure unittest
{
static struct B
{
int val;
bool flag;
@disable this(this);
}
auto b = B();
assert(b.to!string == "B(0, false)");
}|
@wilzbach Added |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks!
Enables conversions to string for structs with a disabled postblit