Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

Bug 7660, incorrect toImpl overload for struct with alias this to class #482

Merged
merged 6 commits into from

2 participants

std/conv.d
@@ -930,7 +930,7 @@ T toImpl(T, S)(S s, in T leftBracket, in T keyval = ":", in T separator = ", ",
/// ditto
T toImpl(T, S)(S s)
- if (is(S : Object) &&
+ if (is(S : Object) && !is(S == struct) &&
@9rnsr Collaborator
9rnsr added a note

I think is(S == class) is better than is(S : Object) && !is(S == struct).

@Dav1dde
Dav1dde added a note

didn't think of that

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
std/conv.d
@@ -959,6 +959,13 @@ unittest
assert(to!string(a) == "null");
a = new A;
assert(to!string(a) == "an A");
+
+ // Bug 7660
+ class C { string toString() { return "C"; } }
+ struct S { C c; alias c this; }
+ S s; s.c = new C();
+ static assert(__traits(compiles, to!string(s)));
@9rnsr Collaborator
9rnsr added a note

This static assert is redundant, because the next run-time assertion implicitly needs that itself is compilable.

@Dav1dde
Dav1dde added a note

I added it to stress, that this bug is fixed and if it occurs again you know it's a regression

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
std/conv.d
@@ -959,6 +959,13 @@ unittest
assert(to!string(a) == "null");
a = new A;
assert(to!string(a) == "an A");
+
+ // Bug 7660
+ class C { string toString() { return "C"; } }
@9rnsr Collaborator
9rnsr added a note

You need adding override keyword for toString.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
std/conv.d
@@ -930,7 +930,7 @@ T toImpl(T, S)(S s, in T leftBracket, in T keyval = ":", in T separator = ", ",
/// ditto
T toImpl(T, S)(S s)
- if (is(S : Object) &&
+ if (is(S == Object) &&
@9rnsr Collaborator
9rnsr added a note

No, == Object and == class are obviously different.
The former returns true only if S equals to Object, and the latter can tests whether S is a kind of class type (not only Object, and user-defined class types).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@9rnsr
Collaborator

Did you run phobos unit test in your local?

@9rnsr
Collaborator

Github doesn't send notify e-mail when you push only commits.
If you add some changes, please add some comment together.

@9rnsr
Collaborator

Ping?

@Dav1dde

(S : Object) → (S == class)

@9rnsr
Collaborator

Now, you can check your pull's result in Pull Tester.
It shows to you the needing of rebasing.

@Dav1dde

Should work now?
I removed the changelog note, since there's no real place for it (no 2.060 section)

@9rnsr
Collaborator

OK. Changelog conflict is resolved.

@9rnsr 9rnsr merged commit 803aa7c into from
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Apr 19, 2012
  1. @Dav1dde

    fixed toImpl conflict

    Dav1dde authored
  2. @Dav1dde

    added tests for bug 7660

    Dav1dde authored
  3. @Dav1dde
  4. @Dav1dde

    reworked fix

    Dav1dde authored
  5. @Dav1dde

    changed toImpl fix as proposed

    Dav1dde authored
  6. @Dav1dde

    removed changelog notes

    Dav1dde authored
This page is out of date. Refresh to see the latest.
Showing with 7 additions and 1 deletion.
  1. +7 −1 std/conv.d
View
8 std/conv.d
@@ -930,7 +930,7 @@ T toImpl(T, S)(S s, in T leftBracket, in T keyval = ":", in T separator = ", ",
/// ditto
T toImpl(T, S)(S s)
- if (is(S : Object) &&
+ if (is(S == class) &&
isSomeString!T)
{
return toStr!T(s);
@@ -959,6 +959,12 @@ unittest
assert(to!string(a) == "null");
a = new A;
assert(to!string(a) == "an A");
+
+ // Bug 7660
+ class C { override string toString() { return "C"; } }
+ struct S { C c; alias c this; }
+ S s; s.c = new C();
+ assert(to!string(s) == "C");
}
/// ditto
Something went wrong with that request. Please try again.