Supplemental changes of druntime/pull/72 - Issue 1824 - Object not const correct #262

Merged
merged 1 commit into from Jul 9, 2012

Conversation

Projects
None yet
4 participants
@jmdavis

This comment has been minimized.

Show comment Hide comment
@jmdavis

jmdavis Sep 18, 2011

Member

I would strongly argue that when const is used to mark functions, it should go after the function and not before. Otherwise, it risks confusion, making it look like the return type is const, when it isn't.

Member

jmdavis commented Sep 18, 2011

I would strongly argue that when const is used to mark functions, it should go after the function and not before. Otherwise, it risks confusion, making it look like the return type is const, when it isn't.

@andralex

This comment has been minimized.

Show comment Hide comment
@andralex

andralex Sep 25, 2011

Owner

This is a big breaking change. We need to do as follows:

  1. The const methods in Object you replaced should have a non-const AND a const version. The non-const version is for backward compatibility.
  2. By default the const method does the work and the non-const method forwards to it.
Owner

andralex commented Sep 25, 2011

This is a big breaking change. We need to do as follows:

  1. The const methods in Object you replaced should have a non-const AND a const version. The non-const version is for backward compatibility.
  2. By default the const method does the work and the non-const method forwards to it.
@jmdavis

This comment has been minimized.

Show comment Hide comment
@jmdavis

jmdavis Sep 25, 2011

Member

Assuming that we're keeping the non-const versions around permanently (which will make some folks happier, I'm sure), we need to make it very clear in the documention somewhere that you need to always override the const version and that if you override the non-const version in any of a class' base classes, you need to make sure that you override the non-const version as well. Otherwise, there will be bugs.

The purist in me would very much like us to only have the const versions in the long run (particularly when you take the risk of bugs when overriding only one of them into account), but from a performance point of view, having both would be prudent.

Member

jmdavis commented Sep 25, 2011

Assuming that we're keeping the non-const versions around permanently (which will make some folks happier, I'm sure), we need to make it very clear in the documention somewhere that you need to always override the const version and that if you override the non-const version in any of a class' base classes, you need to make sure that you override the non-const version as well. Otherwise, there will be bugs.

The purist in me would very much like us to only have the const versions in the long run (particularly when you take the risk of bugs when overriding only one of them into account), but from a performance point of view, having both would be prudent.

This comment has been minimized.

Show comment Hide comment
@ghost

ghost Sep 25, 2011

Agreed with jmdavis, const on the right side (and this is now recommended in the docs since recently).

Btw can inout be used? Or is that still buggy?

ghost commented Sep 25, 2011

Agreed with jmdavis, const on the right side (and this is now recommended in the docs since recently).

Btw can inout be used? Or is that still buggy?

@jmdavis

This comment has been minimized.

Show comment Hide comment
@jmdavis

jmdavis Sep 25, 2011

Member

inout doesn't work yet, and if I understand how it works correctly, it wouldn't help anyway, since inout relates to making the return value having the same constness as the functions arguments IIRC. I'm not sure that that would work at all when the argument which is const or not is this, and even if it did, I don't think that any of Object's functions would gain anything by having their return values be const, regardless of whether this is const or not.

Member

jmdavis commented Sep 25, 2011

inout doesn't work yet, and if I understand how it works correctly, it wouldn't help anyway, since inout relates to making the return value having the same constness as the functions arguments IIRC. I'm not sure that that would work at all when the argument which is const or not is this, and even if it did, I don't think that any of Object's functions would gain anything by having their return values be const, regardless of whether this is const or not.

@JakobOvrum

This comment has been minimized.

Show comment Hide comment
@JakobOvrum

JakobOvrum Sep 25, 2011

Member

If opEquals has to be const in the future, then it won't be usable in many cases when a type is part of a C library wrapper.

For example, in my wrapper of Lua, opEquals pushes the two objects to the Lua stack, uses Lua's comparison function, then pops both objects again (although it's a struct, not a class, the example still applies). This is a neat and fast operation and leaves everything as it was, but the pushing and popping functions cannot be const on their own, so opEquals can't be const either.

opEquals should be allowed to be logically const (but of course, not marked with const).

All the same arguments go for toString, and probably opCmp and opHash as well.

Member

JakobOvrum commented Sep 25, 2011

If opEquals has to be const in the future, then it won't be usable in many cases when a type is part of a C library wrapper.

For example, in my wrapper of Lua, opEquals pushes the two objects to the Lua stack, uses Lua's comparison function, then pops both objects again (although it's a struct, not a class, the example still applies). This is a neat and fast operation and leaves everything as it was, but the pushing and popping functions cannot be const on their own, so opEquals can't be const either.

opEquals should be allowed to be logically const (but of course, not marked with const).

All the same arguments go for toString, and probably opCmp and opHash as well.

This comment has been minimized.

Show comment Hide comment
@ghost

ghost Sep 25, 2011

Funny, I thought I saw inout being used for the this reference before. But this doesn't work:

struct Foo
{
    int test() inout { return 1; }
}

void main()
{
    const(Foo) foo;
    auto x = foo.test();
}

However this works (even though it makes no sense?):

struct Foo
{
    int test() inout { return 1; }
}

void main()
{
    inout(Foo) foo;
    auto x = foo.test();
}

ghost commented Sep 25, 2011

Funny, I thought I saw inout being used for the this reference before. But this doesn't work:

struct Foo
{
    int test() inout { return 1; }
}

void main()
{
    const(Foo) foo;
    auto x = foo.test();
}

However this works (even though it makes no sense?):

struct Foo
{
    int test() inout { return 1; }
}

void main()
{
    inout(Foo) foo;
    auto x = foo.test();
}
@jmdavis

This comment has been minimized.

Show comment Hide comment
@jmdavis

jmdavis Sep 25, 2011

Member

@JakobOvrum For const to work with classes, Object's functions must have const versions, and if that disallows certain things, then that disallows certain things. If it's a C wrapper that's the issue, then you can probably have an extern(C) function marked as const - even though it's really only logically const - which opEquals calls and have it work, but I don't know. However, since structs don't have inheritance to worry about, once Bug #3659 has been sorted out, it should be quite possible to have structs whose opEquals isn't const - but that would mean that such a struct could never be const.

@andrej I don't recall all of the details of inout. I'd have to go look it up in TDPL again to know exactly how it works, but it has never been implemented properly, so what it does and doesn't do when you try and use it right now is more or less meaningless.

Member

jmdavis commented Sep 25, 2011

@JakobOvrum For const to work with classes, Object's functions must have const versions, and if that disallows certain things, then that disallows certain things. If it's a C wrapper that's the issue, then you can probably have an extern(C) function marked as const - even though it's really only logically const - which opEquals calls and have it work, but I don't know. However, since structs don't have inheritance to worry about, once Bug #3659 has been sorted out, it should be quite possible to have structs whose opEquals isn't const - but that would mean that such a struct could never be const.

@andrej I don't recall all of the details of inout. I'd have to go look it up in TDPL again to know exactly how it works, but it has never been implemented properly, so what it does and doesn't do when you try and use it right now is more or less meaningless.

@9rnsr

This comment has been minimized.

Show comment Hide comment
@9rnsr

9rnsr Sep 25, 2011

Member

Current inout isn't implemented properly. I already posted pull request to fix it.
D-Programming-Language/dmd#359

But, we cannot use inout for this issue. Inside inout member function, we should implement equality operation like inside const member function. You never change fields inside inout member function.

Member

9rnsr commented Sep 25, 2011

Current inout isn't implemented properly. I already posted pull request to fix it.
D-Programming-Language/dmd#359

But, we cannot use inout for this issue. Inside inout member function, we should implement equality operation like inside const member function. You never change fields inside inout member function.

@JakobOvrum

This comment has been minimized.

Show comment Hide comment
@JakobOvrum

JakobOvrum Sep 25, 2011

Member

@jmdavis the function (or rather its parameter corresponding to the this reference) wouldn't be const just because you're able to mark it as such when the implementation is linked from elsewhere. Doing that would allow people to break the immutable guarantee and cause all kinds of threading problems and the compiler has no idea. It's basically the same as casting away const.

Yes, opEquals etc. need to have const versions. But removing the old versions is nonsense. There has to be a way for both to co-exist, otherwise you break a lot of code which is actually perfectly fine and correct. The ability to be const correct shouldn't decrease the overall usefulness of operator overloading.

And yes, I know it will work for structs, but the fact that I'm using a struct is an irrelevant detail. Somewhere out there someone is doing the same thing except with a class.

Member

JakobOvrum commented Sep 25, 2011

@jmdavis the function (or rather its parameter corresponding to the this reference) wouldn't be const just because you're able to mark it as such when the implementation is linked from elsewhere. Doing that would allow people to break the immutable guarantee and cause all kinds of threading problems and the compiler has no idea. It's basically the same as casting away const.

Yes, opEquals etc. need to have const versions. But removing the old versions is nonsense. There has to be a way for both to co-exist, otherwise you break a lot of code which is actually perfectly fine and correct. The ability to be const correct shouldn't decrease the overall usefulness of operator overloading.

And yes, I know it will work for structs, but the fact that I'm using a struct is an irrelevant detail. Somewhere out there someone is doing the same thing except with a class.

@jmdavis

This comment has been minimized.

Show comment Hide comment
@jmdavis

jmdavis Sep 25, 2011

Member

@JakobOvrum Of course, using the trick with the C function being marked const would be circumventing the type system, but as long as it's a C wrapper and therefore is never actually immutable, it would work. The only reason that it wouldn't work that I can think of is if D recognizes that C's const is not the same as D's const and disallows the calling of C functions from a D const function.

Even keeping the non-const versions wouldn't work for your use case though, because it would still have to have a const version. A class must be able to have both, or it's not going to work. At best, you could implement the non-const version in the derived class, letting the const version be that of the base class, and never use the object as const, in which case, it would work, but it's asking for trouble, since if you ever created a const instance of that object, it wouldn't work correctly.

And the fact that it's a class or a struct is not an irrelevant detail, because the two have very different characteristics. D does not support logical const in its type system, and since Object's functions need const versions for const to work, any and all classes are going to need to be able to have const versions of those functions to work, and any that can't have const versions of those functions and work will have to be structs. Whether a type is a struct or a class can have a definite impact on how it works, and how a type works can have a definite impact on whether it can be a struct or a class.

Member

jmdavis commented Sep 25, 2011

@JakobOvrum Of course, using the trick with the C function being marked const would be circumventing the type system, but as long as it's a C wrapper and therefore is never actually immutable, it would work. The only reason that it wouldn't work that I can think of is if D recognizes that C's const is not the same as D's const and disallows the calling of C functions from a D const function.

Even keeping the non-const versions wouldn't work for your use case though, because it would still have to have a const version. A class must be able to have both, or it's not going to work. At best, you could implement the non-const version in the derived class, letting the const version be that of the base class, and never use the object as const, in which case, it would work, but it's asking for trouble, since if you ever created a const instance of that object, it wouldn't work correctly.

And the fact that it's a class or a struct is not an irrelevant detail, because the two have very different characteristics. D does not support logical const in its type system, and since Object's functions need const versions for const to work, any and all classes are going to need to be able to have const versions of those functions to work, and any that can't have const versions of those functions and work will have to be structs. Whether a type is a struct or a class can have a definite impact on how it works, and how a type works can have a definite impact on whether it can be a struct or a class.

@JakobOvrum

This comment has been minimized.

Show comment Hide comment
@JakobOvrum

JakobOvrum Sep 25, 2011

Member

@jmdavis it's an irrelevant detail because someone else would be out there using a class, comparing structs and classes here is completely unnecessary.

As for having both const and non-const, you could just implement the const version as throwing if you are not able to satisfy its restrictions. This is pretty common with any kind of interface.

Member

JakobOvrum commented Sep 25, 2011

@jmdavis it's an irrelevant detail because someone else would be out there using a class, comparing structs and classes here is completely unnecessary.

As for having both const and non-const, you could just implement the const version as throwing if you are not able to satisfy its restrictions. This is pretty common with any kind of interface.

@9rnsr

This comment has been minimized.

Show comment Hide comment
@9rnsr

9rnsr Sep 25, 2011

Member

If we need to keep backward compatibility, Object should have two versions: bool opEquals(const Object rhs) const and bool opEquals(Object rhs).
And not need other versions, e.g. bool opEquals(const Object rhs), bool opEquals(Object rhs) const. Because opEquals should be commutative.

But I think adding const opEquals process has one big problem.
Today, all of D users only have mutable opEquals. Adding const opEquals simply into there will cause quite different behavior between const and mutable equality operations. Almost users might not recognize the change.

Member

9rnsr commented Sep 25, 2011

If we need to keep backward compatibility, Object should have two versions: bool opEquals(const Object rhs) const and bool opEquals(Object rhs).
And not need other versions, e.g. bool opEquals(const Object rhs), bool opEquals(Object rhs) const. Because opEquals should be commutative.

But I think adding const opEquals process has one big problem.
Today, all of D users only have mutable opEquals. Adding const opEquals simply into there will cause quite different behavior between const and mutable equality operations. Almost users might not recognize the change.

@JakobOvrum

This comment has been minimized.

Show comment Hide comment
@JakobOvrum

JakobOvrum Sep 25, 2011

Member

A compiler warning if you only implement one but not the other, perhaps? Just throwing out a thought.

Member

JakobOvrum commented Sep 25, 2011

A compiler warning if you only implement one but not the other, perhaps? Just throwing out a thought.

@jmdavis

This comment has been minimized.

Show comment Hide comment
@jmdavis

jmdavis Sep 25, 2011

Member

True, but that's a permanent issue with having both a const and non-const opEquals. It becomes necessary to always declare both, or you're going to have bugs. JakobOvrum's suggestion of a compiler warning would be a good way to deal with that though.

Member

jmdavis commented Sep 25, 2011

True, but that's a permanent issue with having both a const and non-const opEquals. It becomes necessary to always declare both, or you're going to have bugs. JakobOvrum's suggestion of a compiler warning would be a good way to deal with that though.

@9rnsr

This comment has been minimized.

Show comment Hide comment
@9rnsr

9rnsr Sep 25, 2011

Member

I think it's an issue of the attitude of D to class object.

Current D has two points.

  1. Force "Liskov substitution principle" between two mutable class objects. (Today it is not defined against const objects)

    It is implemented by global opEquals function in object module. Current class object equality is always processed that function (It means that member opEquals function never called directly, and it is useless that defining specialized equality overriding like bool opEquals(UserClass rhs)).

  2. Force commutative of equality between two different typed mutable class object. (Today it is not defined against const objects)

    It is implemented to force overriding bool opEquals(Object) against derived classes, and by global function opEquals in object module, if the two objects have different TypeInfos, the result is caluculated by lhs.opEquals(rhs) && rhs.opEquals(lhs).

Member

9rnsr commented Sep 25, 2011

I think it's an issue of the attitude of D to class object.

Current D has two points.

  1. Force "Liskov substitution principle" between two mutable class objects. (Today it is not defined against const objects)

    It is implemented by global opEquals function in object module. Current class object equality is always processed that function (It means that member opEquals function never called directly, and it is useless that defining specialized equality overriding like bool opEquals(UserClass rhs)).

  2. Force commutative of equality between two different typed mutable class object. (Today it is not defined against const objects)

    It is implemented to force overriding bool opEquals(Object) against derived classes, and by global function opEquals in object module, if the two objects have different TypeInfos, the result is caluculated by lhs.opEquals(rhs) && rhs.opEquals(lhs).

@andralex

This comment has been minimized.

Show comment Hide comment
@andralex

andralex Sep 25, 2011

Owner

So, after all this discussion:

  1. inout is a red herring for this particular topic.
  2. We must keep bool opEquals(Object rhs) for backward compatibility.
  3. We must add bool opEquals(const Object rhs) const for allowing people to define comparison of constant objects.
  4. The default implementation of the non-const version must be as follows. This is such that people who only override the const method (as most should) get the right behavior for both const and non-const objects.
    bool opEquals(Object rhs)
    {
        const cThis = this;
        return cThis.opEquals(rhs);
    }

This all restricts what we can do to a narrow path. The disadvantage of the scheme is that people who override the non-const version but don't override the const version will effectively have two distinct methods of comparing objects depending on their constness, which is not recommendable. It seems this is a cost we hate to pay.

Owner

andralex commented Sep 25, 2011

So, after all this discussion:

  1. inout is a red herring for this particular topic.
  2. We must keep bool opEquals(Object rhs) for backward compatibility.
  3. We must add bool opEquals(const Object rhs) const for allowing people to define comparison of constant objects.
  4. The default implementation of the non-const version must be as follows. This is such that people who only override the const method (as most should) get the right behavior for both const and non-const objects.
    bool opEquals(Object rhs)
    {
        const cThis = this;
        return cThis.opEquals(rhs);
    }

This all restricts what we can do to a narrow path. The disadvantage of the scheme is that people who override the non-const version but don't override the const version will effectively have two distinct methods of comparing objects depending on their constness, which is not recommendable. It seems this is a cost we hate to pay.

@9rnsr

This comment has been minimized.

Show comment Hide comment
@9rnsr

9rnsr Sep 25, 2011

Member

I suggest two changes for opEquals consistency.

  1. Object class has only bool opEqua(const Object) const method.

  2. Change object.opEquals(Object, Object) function like follows:

    bool opEquals(T, U)(T lhs, U rhs)
        if (is(T == class) && is(U == class))
    {
        // If aliased to the same object or both null => equal
        if (lhs is rhs) return true;
    
        // If either is null => non-equal
        if (lhs is null || rhs is null) return false;
    
        // If same exact type => one call to method opEquals
        if (typeid(lhs) is typeid(rhs) || typeid(lhs).opEquals(typeid(rhs)))
            return lhs.opEquals(rhs);
    
        static if (is(T == const) || is(U == const)))
        {   // const   == const, mutable == const, const   == mutable
    
            // #1 General case => symmetric calls to method opEquals
            return lhs.opEquals(rhs) && rhs.opEquals(lhs);
        }
        else
        {   // mutable == mutable
    
            // #2-1 require that parameter type is mutable Object version ?
            // == calling
            //          ( T.opEquals(Object) or T.opEquals(const Object) const )
            //      and ( U.opEquals(Object) or U.opEquals(const Object) const )
            return lhs.opEquals( cast(Object)(rhs) ) && rhs.opEquals( cast(Object)(lhs) );
    
            // #2-2 or allow comparing between exact types
            // == allow calling
            //          ( T.opEquals(U) or ... or T.opEquals(const Object) const )
            //      and ( U.opEquals(T) or ... or U.opEquals(const Object) const )
            return lhs.opEquals( rhs ) && rhs.opEquals( lhs );
        }
    }
    

    If one or more side of == are const, they are compared by bool opEquals(const Object) const. (#1)
    If both objects are mutable,

    • #2-1 require opEquals(Object) for mutable comparing.
    • #2-2 allow exact type comparing

    Both allow breaking "Loskov substitution principle".

Advantages:

  • Only necessary Object.opEquals(const Object) const (keep vtable size small)

  • Better breaking of existing code.
    This change prints Error against all current codes:

      "class <i>UserClass</i> use of Object.opEquals(const Object rhs) const hidden by <i>UserClass</i> is deprecated"
    

    I think most of current D codes can be separated two kinds: really breaks constness or "faithful const" opEquals.
    Against former, this change requires defining const version of opEquals. It is right way.
    And against latter, this change requires changing signature to bool opEquals(const Object) const, it is right way, too.

    We must change the code around opEquals somewhere sometime. Delaying it will only increase the debt.

Disadvantages:

  • A little complexity of comparing equality.
Member

9rnsr commented Sep 25, 2011

I suggest two changes for opEquals consistency.

  1. Object class has only bool opEqua(const Object) const method.

  2. Change object.opEquals(Object, Object) function like follows:

    bool opEquals(T, U)(T lhs, U rhs)
        if (is(T == class) && is(U == class))
    {
        // If aliased to the same object or both null => equal
        if (lhs is rhs) return true;
    
        // If either is null => non-equal
        if (lhs is null || rhs is null) return false;
    
        // If same exact type => one call to method opEquals
        if (typeid(lhs) is typeid(rhs) || typeid(lhs).opEquals(typeid(rhs)))
            return lhs.opEquals(rhs);
    
        static if (is(T == const) || is(U == const)))
        {   // const   == const, mutable == const, const   == mutable
    
            // #1 General case => symmetric calls to method opEquals
            return lhs.opEquals(rhs) && rhs.opEquals(lhs);
        }
        else
        {   // mutable == mutable
    
            // #2-1 require that parameter type is mutable Object version ?
            // == calling
            //          ( T.opEquals(Object) or T.opEquals(const Object) const )
            //      and ( U.opEquals(Object) or U.opEquals(const Object) const )
            return lhs.opEquals( cast(Object)(rhs) ) && rhs.opEquals( cast(Object)(lhs) );
    
            // #2-2 or allow comparing between exact types
            // == allow calling
            //          ( T.opEquals(U) or ... or T.opEquals(const Object) const )
            //      and ( U.opEquals(T) or ... or U.opEquals(const Object) const )
            return lhs.opEquals( rhs ) && rhs.opEquals( lhs );
        }
    }
    

    If one or more side of == are const, they are compared by bool opEquals(const Object) const. (#1)
    If both objects are mutable,

    • #2-1 require opEquals(Object) for mutable comparing.
    • #2-2 allow exact type comparing

    Both allow breaking "Loskov substitution principle".

Advantages:

  • Only necessary Object.opEquals(const Object) const (keep vtable size small)

  • Better breaking of existing code.
    This change prints Error against all current codes:

      "class <i>UserClass</i> use of Object.opEquals(const Object rhs) const hidden by <i>UserClass</i> is deprecated"
    

    I think most of current D codes can be separated two kinds: really breaks constness or "faithful const" opEquals.
    Against former, this change requires defining const version of opEquals. It is right way.
    And against latter, this change requires changing signature to bool opEquals(const Object) const, it is right way, too.

    We must change the code around opEquals somewhere sometime. Delaying it will only increase the debt.

Disadvantages:

  • A little complexity of comparing equality.
@9rnsr

This comment has been minimized.

Show comment Hide comment
@9rnsr

9rnsr Sep 25, 2011

Member

@andralex My suggestion can prompt to all D users to rewriting code by deprecated error message.
I think we should pay the cost as soon as possible. The reason of having increased it is we have hated to pay it.

Member

9rnsr commented Sep 25, 2011

@andralex My suggestion can prompt to all D users to rewriting code by deprecated error message.
I think we should pay the cost as soon as possible. The reason of having increased it is we have hated to pay it.

@9rnsr

This comment has been minimized.

Show comment Hide comment
@9rnsr

9rnsr Sep 25, 2011

Member

Do not afraid the cost of changes. Hating it will only increase the future liabilities.

Member

9rnsr commented Sep 25, 2011

Do not afraid the cost of changes. Hating it will only increase the future liabilities.

@andralex

This comment has been minimized.

Show comment Hide comment
@andralex

andralex Sep 25, 2011

Owner

Your implementation won't work without Object defining a non-const method.

The problem is that people defined this all over the place:

bool opEquals(Object rhs)

We can't afford to leave them in the cold.

Owner

andralex commented Sep 25, 2011

Your implementation won't work without Object defining a non-const method.

The problem is that people defined this all over the place:

bool opEquals(Object rhs)

We can't afford to leave them in the cold.

@9rnsr

This comment has been minimized.

Show comment Hide comment
@9rnsr

9rnsr Sep 25, 2011

Member

No, it will work.

// #2-1
return lhs.opEquals( cast(Object)(rhs) ) && rhs.opEquals( cast(Object)(lhs) );

In this case, the type of lhs is T, not Object. The mutable opEquals is seached from exact type T and U, and if they are not exist, the calling is fallbacked to Object.opEquals(const Object) const.
(Don't forget that mutable object is implicitly convertible to const.)

Member

9rnsr commented Sep 25, 2011

No, it will work.

// #2-1
return lhs.opEquals( cast(Object)(rhs) ) && rhs.opEquals( cast(Object)(lhs) );

In this case, the type of lhs is T, not Object. The mutable opEquals is seached from exact type T and U, and if they are not exist, the calling is fallbacked to Object.opEquals(const Object) const.
(Don't forget that mutable object is implicitly convertible to const.)

@9rnsr

This comment has been minimized.

Show comment Hide comment
@9rnsr

9rnsr Sep 25, 2011

Member

OK. I tried my suggestion in local, and succeeded to print deprecated error message against following code:

class C
{
    bool opEquals(Object o) { return false; }
    bool opEquals(Object o) const { return false; }
    // test.d(132): Error: class test.C use of object.Object.opEquals(const(Object) o) 
    // hidden by C is deprecated

    bool opEquals(const Object o) { return false; }
    // test.d(134): Error: function test.C.opEquals of type bool(const const(Object) o) 
    // overrides but is not covariant with object.Object.opEquals of type const bool(const(Object) o)

    bool opEquals(const Object o) const { return false; }
    // OK
}

There is only one problem.

This error message is depends on "Hidden func error". If we specifiy -d switch in command line, the message is suppressed, and code will silently break.

Member

9rnsr commented Sep 25, 2011

OK. I tried my suggestion in local, and succeeded to print deprecated error message against following code:

class C
{
    bool opEquals(Object o) { return false; }
    bool opEquals(Object o) const { return false; }
    // test.d(132): Error: class test.C use of object.Object.opEquals(const(Object) o) 
    // hidden by C is deprecated

    bool opEquals(const Object o) { return false; }
    // test.d(134): Error: function test.C.opEquals of type bool(const const(Object) o) 
    // overrides but is not covariant with object.Object.opEquals of type const bool(const(Object) o)

    bool opEquals(const Object o) const { return false; }
    // OK
}

There is only one problem.

This error message is depends on "Hidden func error". If we specifiy -d switch in command line, the message is suppressed, and code will silently break.

@9rnsr

This comment has been minimized.

Show comment Hide comment
@9rnsr

9rnsr Sep 29, 2011

Member

Change prefix const to suffix.
And add feature for opEquals migration. See D-Programming-Language/druntime#72 .

Member

9rnsr commented Sep 29, 2011

Change prefix const to suffix.
And add feature for opEquals migration. See D-Programming-Language/druntime#72 .

@9rnsr

This comment has been minimized.

Show comment Hide comment
@9rnsr

9rnsr Oct 15, 2011

Member

Updated.
New points:

  • From the discussions in newsgroup, enable direct comparison with mutable opEquals.
    If disable it, change here to latter.
  • Enable interface comparison for fixing issue 4088.
  • Add changes for fixing issue 3789. It requires const equality comparison for member classes.
Member

9rnsr commented Oct 15, 2011

Updated.
New points:

  • From the discussions in newsgroup, enable direct comparison with mutable opEquals.
    If disable it, change here to latter.
  • Enable interface comparison for fixing issue 4088.
  • Add changes for fixing issue 3789. It requires const equality comparison for member classes.
@9rnsr

This comment has been minimized.

Show comment Hide comment
@9rnsr

9rnsr Apr 22, 2012

Member

Rebased.

Member

9rnsr commented Apr 22, 2012

Rebased.

@ghost ghost assigned andralex Jul 9, 2012

@andralex

This comment has been minimized.

Show comment Hide comment
@andralex

andralex Jul 9, 2012

Owner

Ah, we need to wait for the dmd commit first and foremost. @WalterBright, we need to synchronize and pull dmd, druntime, and phobos simultaneously.

Owner

andralex commented Jul 9, 2012

Ah, we need to wait for the dmd commit first and foremost. @WalterBright, we need to synchronize and pull dmd, druntime, and phobos simultaneously.

andralex added a commit that referenced this pull request Jul 9, 2012

Merge pull request #262 from 9rnsr/constApply
Supplemental changes of druntime/pull/72 - Issue 1824 - Object not const correct

@andralex andralex merged commit 99671b3 into dlang:master Jul 9, 2012

9rnsr added a commit to 9rnsr/phobos that referenced this pull request Jul 17, 2012

Revert "Merge pull request #262 from 9rnsr/constApply"
This reverts commit 99671b3, reversing
changes made to 5fa37c1.

kuettler pushed a commit to kuettler/phobos that referenced this pull request Feb 6, 2018

Merge pull request #262 from MartinNowak/merge_stable
Merge remote-tracking branch 'upstream/stable' into merge_stable
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment