http://d.puremagic.com/issues/show_bug.cgi?id=5354



--- Comment #10 from bearophile_hugs@eml.cc 2010-12-15 23:03:16 PST ---
(In reply to comment #9)

> +++

You may also actually vote for the bug 3813 :-)

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------

http://d.puremagic.com/issues/show_bug.cgi?id=5354


Rob Jacques <sandford@jhu.edu> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Priority|P2                          |P3
                 CC|                            |sandford@jhu.edu
           Platform|x86                         |All
         OS/Version|Linux                       |All
           Severity|blocker                     |major


--- Comment #11 from Rob Jacques <sandford@jhu.edu> 2010-12-30 12:26:10 PST ---
I've decreased the importance of this as there are work arounds. As for my thoughts, this bug is causing a major loss of function in my update to std.variant and is going to cause major issues with any type that defines/needs a permissive opDispatch. Conceptually, I believe formatValue makes a major mistake by assuming that just because a type satisfies isRange, that it is, in fact a range. So I believe that if toString is present, it should take priority. I don't see a problem with ranges that return their own types (i.e. trees) or the current method of formating in general. (Note that the patch only deals with ElementType!T == T and not the toString issue)

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------

http://d.puremagic.com/issues/show_bug.cgi?id=5354


Andrei Alexandrescu <andrei@metalanguage.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |ASSIGNED
                 CC|                            |andrei@metalanguage.com
         AssignedTo|nobody@puremagic.com        |andrei@metalanguage.com


-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------

http://d.puremagic.com/issues/show_bug.cgi?id=5354



--- Comment #12 from Andrei Alexandrescu <andrei@metalanguage.com> 2011-01-23 13:28:48 PST ---
There are good arguments for going either way wrt the relative priority of toString and range interface.

Using toString by default is in a way the "right" thing to do as it's the default formatting for all non-range objects. For class objects, I oppose distinguishing between introduced toString and inherited toString; that goes against what inheritance is about.

There are a few problems with toString, however. It needs to format everything in memory before writing, which makes formatting large ranges slow. Also, if you have a range but you want to use toString it's easy to simply write r.toString, whereas there is no simple method to say "even though this range has toString, please use the range functions to format it".

It's true that there's a danger of accidental conformance to inputRange. But at this point the input range troika is about as widespread as toString, so I don't think that's a major risk.

I have the changes proposed by Nick with a few edits in my tree. Unless a solid argument comes forward, I'll commit them soon.

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------

http://d.puremagic.com/issues/show_bug.cgi?id=5354



--- Comment #13 from Denis Derman <denis.spir@gmail.com> 2011-01-23 14:25:33 PST ---
(In reply to comment #12)
> There are good arguments for going either way wrt the relative priority of toString and range interface.
> 
> Using toString by default is in a way the "right" thing to do as it's the default formatting for all non-range objects. For class objects, I oppose distinguishing between introduced toString and inherited toString; that goes against what inheritance is about.

Actually, my point is not about which of toString or range format should have precedence. Rather that a programmer does defines toString in purpose: for it to be used by builtin routines like write* functions. Ignoring it is not acceptable. Moreover, there is no reason that a _default_ format fits specific needs.

About toString issues such as memory usage, I do agree. They are planned to be solved with writeTo. Then, writeTo defined by the programmer should be used for any kind of object output, just like currently toString should be used when defined.

Finally, as explained above, letting range default format shortcut custom toString does not permit outputting any range which ElementType is itself. (infinite loop bug)

Denis

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------

http://d.puremagic.com/issues/show_bug.cgi?id=5354



--- Comment #14 from Rob Jacques <sandford@jhu.edu> 2011-01-23 17:54:37 PST ---
(In reply to comment #12)
> It's true that there's a danger of accidental conformance to inputRange. But at this point the input range troika is about as widespread as toString, so I don't think that's a major risk.

It's not a major risk in _hand-written_ code, but in generic code it's a major risk due to opDispatch. See comment 11.

Reading over Nick's suggestions (comment 2), he predicates point 2 on the assumption that programmer intent is unclear in the case of classes, due to inheritance of toString from Object and then argues that structs, which do have clear programmer intent should behave like class for uniformity. However, in D2 it is trivial to determine if a programmer of a class has actually implemented a custom toString routine or not (see the code listing below). Therefore, I'd recommend formatValue to prioritize the use of custom toString routines when they exist.

bool hasCustomToStringHelper(T)() {
    foreach(type; TypeTuple!(T,TransitiveBaseTypeTuple!T) ) {
        if(is(Unqual!type == Object))
            return false;
        foreach(member;__traits(derivedMembers,type)) {
            if(member == "toString")
                return true;
        }
    }
    return false;
}
template hasCustomToString(T) {
    static if( __traits(hasMember,T,"toString") ) {
        static if( is(T==class) ) {
            enum hasCustomToString = hasCustomToStringHelper!T;
        } else { // structs, etc
            enum hasCustomToString = true;
        }
    } else {
        enum hasCustomToString = false;
    }
}

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------

http://d.puremagic.com/issues/show_bug.cgi?id=5354


Max Samukha <samukha@voliacable.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |samukha@voliacable.com


--- Comment #15 from Max Samukha <samukha@voliacable.com> 2011-01-24 05:23:19 PST ---
(In reply to comment #14)

> 
> bool hasCustomToStringHelper(T)() {
>     foreach(type; TypeTuple!(T,TransitiveBaseTypeTuple!T) ) {
>         if(is(Unqual!type == Object))
>             return false;
>         foreach(member;__traits(derivedMembers,type)) {
>             if(member == "toString")
>                 return true;
>         }
>     }
>     return false;
> }
> template hasCustomToString(T) {
>     static if( __traits(hasMember,T,"toString") ) {
>         static if( is(T==class) ) {
>             enum hasCustomToString = hasCustomToStringHelper!T;
>         } else { // structs, etc
>             enum hasCustomToString = true;
>         }
>     } else {
>         enum hasCustomToString = false;
>     }
> }

This wouldn't work because the dynamic type of the object being formatted can differ from the static type the template was instantiated with. Even if the compile-time check won't detect the overriden toString, we would still need to perform the check at run-time. The easiest way is to compare the address of the passed-in object's toString to that of Object.toString. Unfortunately that won't work for objects coming from DLLs since they have distinct Object.toString functions. So the correct way is to do proper lookup by name via run-time reflection.

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------

http://d.puremagic.com/issues/show_bug.cgi?id=5354



--- Comment #16 from Denis Derman <denis.spir@gmail.com> 2011-01-24 05:40:24 PST ---
(In reply to comment #15)

> This wouldn't work because the dynamic type of the object being formatted can differ from the static type the template was instantiated with. Even if the compile-time check won't detect the overriden toString, we would still need to perform the check at run-time. The easiest way is to compare the address of the passed-in object's toString to that of Object.toString. Unfortunately that won't work for objects coming from DLLs since they have distinct Object.toString functions. So the correct way is to do proper lookup by name via run-time reflection.

We may find a way to tell apart, for classes, builtin from toString from custom
one. If we cannot, then in doubt toString must have precedence for classes.
Note there is another bug: one currently cannot implement a range interface on
a class that defines toString (because formatValue template constraints are not
mutually exclusive).
In any case, struct toString must have precedence.

It is an obvious choice: programmers explicitely define toString (later,
writeTo) precisely for that ;-)

Denis

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------

http://d.puremagic.com/issues/show_bug.cgi?id=5354



--- Comment #17 from Max Samukha <samukha@voliacable.com> 2011-01-24 06:27:00 PST ---
(In reply to comment #16)
> 
> It is an obvious choice: programmers explicitely define toString (later,
> writeTo) precisely for that ;-)
> 
> Denis

I agree that user-specified toString should take precedence over the ranges. What I wanted to point out is that static checks for classes are not enough.

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------

http://d.puremagic.com/issues/show_bug.cgi?id=5354


Kenji Hara <k.hara.pg@gmail.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Keywords|                            |patch


--- Comment #18 from Kenji Hara <k.hara.pg@gmail.com> 2011-10-20 10:10:38 PDT ---
We can check the toString method is overridden like follows:

string delegate() dg = &obj.toString;
auto overridden = dg.funcptr != &Object.toString;

https://github.com/D-Programming-Language/phobos/pull/298

For class range objects, if the toString method is actually overridden, use it.

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------