Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Overload from opDispatch ignored in WithStatement #19436

Open
dlangBugzillaToGithub opened this issue May 16, 2018 · 8 comments
Open

Overload from opDispatch ignored in WithStatement #19436

dlangBugzillaToGithub opened this issue May 16, 2018 · 8 comments

Comments

@dlangBugzillaToGithub
Copy link

Simen Kjaeraas reported this on 2018-05-16T14:29:16Z

Transferred from https://issues.dlang.org/show_bug.cgi?id=18866

CC List

  • John Hall
  • RazvanN
  • Mike Franklin
  • Paul Backus

Description

When opDispatch would introduce an overload to an existing function, and is being invoked in a WithStatement, the global function is used in its stead:

string fun1() { return "global"; }
string fun2() { return "global"; }
struct S {
    string opDispatch(string name)() {
        return "struct";
    }
    string fun2() {
        return "struct";
    }
}

unittest {
    import std.stdio;
    with (S()) {
        // opDispatch overloading global:
        writeln("fun1: ", fun1()); // Prints 'global'
        // Member function overloading global:
        writeln("fun2: ", fun2()); // Prints 'struct'
        // opDispatch, no overloading:
        writeln("fun3: ", fun3()); // Prints 'struct'
    }
}

Expected behavior here is that all three function calls print 'struct'.
@dlangBugzillaToGithub
Copy link
Author

slavo5150 commented on 2018-05-16T14:33:18Z

This may have been introduced by https://github.com/dlang/dmd/pull/7356

@dlangBugzillaToGithub
Copy link
Author

slavo5150 commented on 2018-05-16T14:44:04Z

Or actually it may be this PR that introduced the issue:  https://github.com/dlang/dmd/pull/6439

@dlangBugzillaToGithub
Copy link
Author

razvan.nitu1305 commented on 2018-05-28T12:07:01Z

I'm not sure if this bug report is valid. The current behavior might be a future.
If this "bug" would be fixed there would be absolutely no way of calling fun1() from within the WithStatement body and that IMHO is an arbitrary limitation.

As things stand now, the compiler first tries to resolve fun1 as a member of S1 and if that's not possible it goes up the enclosing scope. If the chain of scopes is over and fun1 still wasn't resolved then opDispatch is called. In my opinion this makes a lot more sense then calling opDispatch for every method that is not defined in the struct.

@dlangBugzillaToGithub
Copy link
Author

simen.kjaras commented on 2018-05-28T12:24:49Z

> there would be absolutely no way of calling fun1() from within the WithStatement body

Sure there would. Assuming the same code as in comment 0, you would call the global fun2 using .fun2();. You can test this by duplicating the line that calls fun2 and adding a period - it will print 'global'. There's no reason to assume that wouldn't work if an overload was available via opDispatch.

@dlangBugzillaToGithub
Copy link
Author

john.michael.hall commented on 2018-11-16T15:23:38Z

I think a case can be made for fixing this. 

At a minimum, I think the spec is vague in this instance. On opDispatch it just says that things not found will be forwarded based on opDispatch. On the with statement it makes no reference to opDispatch, suggesting that opDispatch should happen first, rather than last. 

Moreover, the spec says "This is to reduce the risk of inadvertant breakage of with statements when new members are added to the object declaration." Below is the example from the documentation discussed in one of the PRs mentioned above. The behavior is totally changed if you add in a global function, e.g. 
void f() { writeln("f global"); }
In other words, the way it currently operates raises the risk of inadvertant breakage when new global functions are added. 

So I think a case can be made for fixing this, or at least making the spec clearer about how with statements interact with opDispatch to make clear how it currently works.



---
import std.stdio;

struct Foo
{
    void opDispatch(string name)()
    {
        mixin("writeln(\"Foo.opDispatch!" ~ name ~ "\");");
    }
}
 struct Bar
{
    // `Bar` does not implement `f()` or `opDispatch`
}
 void main()
{
    Foo foo;
    Bar bar;
     with(foo)
    {
        f();       // prints "Foo.opDispatch!f"
        with(bar)
        {
            f();   // Prior to this Release: Error: undefined identifer `f`
                   // Starting with  this release: Prints "Foo.opDispatch!f".
                   // `f`'s resolution is forwarded up the scope hierarchy.
        }
    }
}
---

@dlangBugzillaToGithub
Copy link
Author

simen.kjaras commented on 2018-11-16T17:56:27Z

(In reply to John Hall from comment #5)

You're right that a new global function will shadow opDispatch, but with the fix the exact opposite problem will appear, so it's not all that simple.

@dlangBugzillaToGithub
Copy link
Author

john.michael.hall commented on 2018-11-16T18:15:58Z

I get that. The point I wanted to highlight was that even if it's not changed  at least the spec can be beefed up.

@dlangBugzillaToGithub
Copy link
Author

snarwin+bugzilla commented on 2023-02-17T21:00:46Z

Another example, from the forums:

---
enum Suit { clubs, spades, hearts, diamonds }

struct Card {
  void opDispatch(string s)(.Suit) {}
}

void main() {
  Card c;
  with (c) Suit = .Suit.diamonds; // Error: `Suit` is not an lvalue and cannot be modified
}
---

It doesn't seem to matter whether the existing symbol is a function, a type, a variable, or anything else; or whether it's declared at module scope or locally. As long as any symbol with the requested name exists in any enclosing scope, the with statement will not call opDispatch.

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

No branches or pull requests

1 participant