fix(dsymbolsem): override methods should have semantic logic on attributes

[ljmf00]

Fix issue 23138.

Signed-off-by: Luís Ferreira contact@lsferreira.net

[dlang-bot]

Thanks for your pull request, @ljmf00!

Bugzilla references

Auto-close	Bugzilla	Severity	Description
✓	23138	normal	Overrides of member functions of an inherited class ignores attribute "downcast"

Testing this PR locally

If you don't have a local development environment setup, you can use Digger to test this PR:

dub run digger -- build "master + dmd#14166"

[ljmf00]

Just cross referencing this with #12934 to remind myself to implement this for new throw attribute.

[RazvanN7]

What about other attributes?

[ljmf00]

What about other attributes?

I can't think of other conflicts here. @nogc can't override to some @gc attribute, because there is no such attribute, atm. This is applied to pure and nothrow too except when throw is a thing. Can you help me find other cases where this might pass and should fail with some other attribute?

[RazvanN7]

My expectation is that if you override a method and you do not specifically add any attributes, then it should be considered that you are trying to lower the restrictions.

For example:

class A
{
     void fun() pure @nogc @safe nothrow {}
}

class B
{
    override void fun() {}
}

I think it is surprising that the compiler simply thinks that fun is nothrow pure @nogc @safe since it is not annotated that way. I understand that it is not allowed to loosen restrictions in overriding methods, but it might be surprising if someone inherits from a library class and does not annotate the method in any way, but the compiler then complains inside the body about violations of attributes that the user did not write. The alternative should be that the user is forced to spell out all the attributes even if it is redundant. It makes for more readable and less surprising code.

To be clear. I think that in my above example, B.fun should issue an error reminding the user that the method he is trying to overwrite needs to follow certain restrictions.

[ljmf00]

[...]

To be clear. I think that in my above example, B.fun should issue an error reminding the user that the method he is trying to overwrite needs to follow certain restrictions.

I think you missed class B : A but I understand what you mean.

I thought the other way around though, since for me I see it as: the default attributes are applied and fetched from override, then the explicit ones try to override the default signature.

Specification-wise, I don't think we have this explicit, but if someone can point me out, would be cool. Even though, I believe that proposal will bring a breaking change.

This change already behaves in pair with the function body semantics so no breaking change. In practice, this fails faster or fails when the function body doesn't have something contrary to specified in the base attributes, which will make the function body semantics more complete.

[RazvanN7]

Ok, sure, my comment should block the addition of this PR. If more people agree with me we can fix this in subsequent PRs.