Issue 8631 - illegal overrides accepted #1450

Merged
merged 1 commit into from Jan 19, 2013

Conversation

Projects
None yet
2 participants

WalterBright added a commit that referenced this pull request Jan 19, 2013

Merge pull request #1450 from 9rnsr/fix8631
Issue 8631 - illegal overrides accepted

@WalterBright WalterBright merged commit 199fc70 into dlang:master Jan 19, 2013

1 check passed

default Pass: 10
Details
+/*
+TEST_OUTPUT:
+---
+fail_compilation/fail8631.d(14): Error: function fail8631.D.foo does not override any function, did you mean to override 'fail8631.B.foo'?

This comment has been minimized.

Show comment Hide comment
@ghost

ghost Jan 20, 2013

Note how the error message is uninformative. You're trying to override foo and then it asks you if you wanted to override foo. If it's only a mismatch of attributes and not of the name of the function we should avoid calling the spellchecker and instead error that foo can't override immutable or const with const shared.

@ghost

ghost Jan 20, 2013

Note how the error message is uninformative. You're trying to override foo and then it asks you if you wanted to override foo. If it's only a mismatch of attributes and not of the name of the function we should avoid calling the spellchecker and instead error that foo can't override immutable or const with const shared.

@ghost

This comment has been minimized.

Show comment Hide comment
@ghost

ghost Jan 24, 2013

Is the following expected?

class B
{
    int foo() immutable { return 2; }
    int foo() const { return 2; }
}

class D : B
{
    override int foo() const shared { return 2; }
    override int foo() const { return 2; }
}

No compilation error. But if you introduce an immutable override:

class B
{
    int foo() immutable { return 2; }
    int foo() const { return 2; }
}

class D : B
{
    override int foo() immutable { return 2; }
    override int foo() const shared { return 2; }  // line 22
    override int foo() const { return 2; }
}

Then the error is on the const shared method.

test.d(22): Error: function test.D.foo does not override any function, did you mean to override 'test.B.foo'?

ghost commented Jan 24, 2013

Is the following expected?

class B
{
    int foo() immutable { return 2; }
    int foo() const { return 2; }
}

class D : B
{
    override int foo() const shared { return 2; }
    override int foo() const { return 2; }
}

No compilation error. But if you introduce an immutable override:

class B
{
    int foo() immutable { return 2; }
    int foo() const { return 2; }
}

class D : B
{
    override int foo() immutable { return 2; }
    override int foo() const shared { return 2; }  // line 22
    override int foo() const { return 2; }
}

Then the error is on the const shared method.

test.d(22): Error: function test.D.foo does not override any function, did you mean to override 'test.B.foo'?
@9rnsr

This comment has been minimized.

Show comment Hide comment
@9rnsr

9rnsr Jan 25, 2013

Member

@AndrejMitrovic : Yes, it is expected.

  • In the first example, D.foo() const shared overrides B.foo() immutable by covariant this overriding.
    (Hmm, it is yet not documented in website. I'll post a pull request for that)
  • In the second example, both of B.foos are overridden in D with exact match, and it is preferred than covariant match.

Therefore, D.foo() const shared will cause an "illegal override" error.

Member

9rnsr commented Jan 25, 2013

@AndrejMitrovic : Yes, it is expected.

  • In the first example, D.foo() const shared overrides B.foo() immutable by covariant this overriding.
    (Hmm, it is yet not documented in website. I'll post a pull request for that)
  • In the second example, both of B.foos are overridden in D with exact match, and it is preferred than covariant match.

Therefore, D.foo() const shared will cause an "illegal override" error.

@ghost

This comment has been minimized.

Show comment Hide comment
@ghost

ghost Jan 25, 2013

Ok. I'll make a separate pull to make the error message better (see my comment here).

ghost commented Jan 25, 2013

Ok. I'll make a separate pull to make the error message better (see my comment here).

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