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

[SDC] Error with inheriting nested classes in nested functions #18964

Open
dlangBugzillaToGithub opened this issue Mar 28, 2015 · 8 comments
Open

Comments

@dlangBugzillaToGithub
Copy link

Shammah Chancellor reported this on 2015-03-28T16:57:30Z

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

CC List

  • ag0aep6g
  • deadalnix
  • yebblies

Description

```
> rdmd test0165.d
test0165.d(22): Error: class test0165.voldemort.basilisk.GinnyWeasley is nested within basilisk, but super class MarvoloRiddle is nested within voldemort
```


```test0165.d
//T compiles:yes
//T has-passed:yes
//T retval:41
// voldemort class with inheritance and 2 different contexts.

auto voldemort() {
	uint a = 7;
	
	class MarvoloRiddle {
		uint b;
		
		this(uint b) {
			this.b = b + a++;
		}
		
		auto foo() {
			return a + b;
		}
	}
	
	auto basilisk(uint c) {
		class GinnyWeasley : MarvoloRiddle {
			this(uint b) {
				a += c++;
				this.b = b + a++;
			}
			
			auto bar() {
				return foo() + a + c;
			}
		}
		
		return new GinnyWeasley(5);
	}
	
	return basilisk(3);
}

auto buzz(V)(V v) {
	return v.bar();
}

int main() {
	auto v = voldemort();
	return buzz(v);
}

```
@dlangBugzillaToGithub
Copy link
Author

k.hara.pg commented on 2015-03-29T04:21:10Z

I'm not sure why SDC accepts the code.

The following reduced case is correctly rejected by dmd, because ba.foo() won't work expectedly. Maybe SDC generates wrong code?

void main()
{
    uint x = 7;

    class A
    {
        auto foo()
        {
            return x;
        }
    }

    auto makeB(uint y)
    {
        class B : A
        {
            auto bar()
            {
                return foo() + y;
            }
        }

        return new B(5);
    }

    A a = new A();
    assert(a.foo() == 7);

    auto b = makeB(3);
    assert(b.bar() == 10);

    A ba = b;
    assert(ba.foo() == 7);
}

@dlangBugzillaToGithub
Copy link
Author

shammah.chancellor commented on 2015-03-29T04:55:25Z

Your code has a bug?

return new B(5);

Should be:

return new B();


With that, your example compiles and works just fine on SDC.


(In reply to Kenji Hara from comment #1)
> I'm not sure why SDC accepts the code.
> 
> The following reduced case is correctly rejected by dmd, because ba.foo()
> won't work expectedly. Maybe SDC generates wrong code?
> 
> void main()
> {
>     uint x = 7;
> 
>     class A
>     {
>         auto foo()
>         {
>             return x;
>         }
>     }
> 
>     auto makeB(uint y)
>     {
>         class B : A
>         {
>             auto bar()
>             {
>                 return foo() + y;
>             }
>         }
> 
>         return new B(5);
>     }
> 
>     A a = new A();
>     assert(a.foo() == 7);
> 
>     auto b = makeB(3);
>     assert(b.bar() == 10);
> 
>     A ba = b;
>     assert(ba.foo() == 7);
> }

@dlangBugzillaToGithub
Copy link
Author

k.hara.pg commented on 2015-03-29T05:04:43Z

(In reply to Shammah Chancellor from comment #2)
> Your code has a bug?
> 
> return new B(5);
> 
> Should be:
> 
> return new B();

Yes. Sorry.

> With that, your example compiles and works just fine on SDC.

Hmm, interesting. SDC might use a thunk to get valid context pointer for base classes from the instantiated context.
But it would need some runtime cost and additional vtbl entry. I don't have any knowledge about the SDC backend, but the ABI around nested classes would be different from dmd's.

@dlangBugzillaToGithub
Copy link
Author

k.hara.pg commented on 2015-03-29T11:09:22Z

Maybe SDC inserts a hidden field to access enclosing scope in each derived classes?

void main() {
    uint x = 7;

    class A {
        int fieldA;
        auto foo() { return x; }
    }

    auto makeB(uint y) {
        class B : A {
            int fieldB;
            auto bar() { return foo() + y; }
        }

        return new B(5);
    }

    A a = new A();
    assert(a.foo() == 7);

    auto b = makeB(3);
    alias B = typeof(b);
    assert(b.bar() == 10);

    pragma(msg, A.fieldA.offsetof);
    pragma(msg, __traits(classInstanceSize, A));
    pragma(msg, B.fieldB.offsetof);
    pragma(msg, __traits(classInstanceSize, B));
}

If B.fieldB.offsetof + int.sizeof + (void*).sizeof <= __traits(classInstanceSize, B), both B and A have their own hidden fields.

Note that it's intentionally disallowed by dmd. With dmd, implicitly insertion of hidden field is limited at most once. At best, it's an incompatibility between dmd and SDC.

@dlangBugzillaToGithub
Copy link
Author

shammah.chancellor commented on 2015-03-29T17:06:59Z

I'm not sure about that part of SDC.  I will investigate and see if deadalnix has some input here.

@dlangBugzillaToGithub
Copy link
Author

deadalnix commented on 2015-03-29T18:27:37Z

(In reply to Shammah Chancellor from comment #5)
> I'm not sure about that part of SDC.  I will investigate and see if
> deadalnix has some input here.

SDC does it by sticking one context in the base and one context in the child.

@dlangBugzillaToGithub
Copy link
Author

k.hara.pg commented on 2015-04-12T12:52:51Z

(In reply to deadalnix from comment #6)
> (In reply to Shammah Chancellor from comment #5)
> > I'm not sure about that part of SDC.  I will investigate and see if
> > deadalnix has some input here.
> 
> SDC does it by sticking one context in the base and one context in the child.

It's definitely different. dmd inserts at most only one context over the all derived classes. The second context pointer in the child class need to be error in SDC.

@dlangBugzillaToGithub
Copy link
Author

deadalnix commented on 2015-04-12T21:03:15Z

(In reply to Kenji Hara from comment #7)
> (In reply to deadalnix from comment #6)
> > (In reply to Shammah Chancellor from comment #5)
> > > I'm not sure about that part of SDC.  I will investigate and see if
> > > deadalnix has some input here.
> > 
> > SDC does it by sticking one context in the base and one context in the child.
> 
> It's definitely different. dmd inserts at most only one context over the all
> derived classes. The second context pointer in the child class need to be
> error in SDC.

I see no reason for this limitation.

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