[REG2.071] Issue 15984 - Interface contracts retrieve garbage instead of parameters

[9rnsr]

No description provided.

[dlang-bot]

Thanks for your pull request, @9rnsr!

Bugzilla references

Auto-close	Bugzilla	Description
✓	15984	[REG2.071] Interface contracts retrieve garbage instead of parameters

[WalterBright]

Why does a new field have to be added to fix a regression? What broke this? Why does fixing a regression require the addition of a lot of logic that was never there before?

[9rnsr]

Why does a new field have to be added to fix a regression?
What broke this?
Why does fixing a regression require the addition of a lot of logic that was never there before?

The purpose of new logic is to fill the insufficiency of original implemenatation for interface contract feature.

Explanation:

interface I
{
    void foo(int x)
    in  // __I_foo_require(void* vthis, size_t N)
    {
        // in here, *vthis should be C.foo.vthis, it's class C instance
        // when __I_foo_require is called from C.foo.

        // By using it, accessing x becomes: *cast(int*)(&vthis + (void*).sizeof)
        int n = x;

        // To get correct reference to I, we should add a runtime offset N from C to I.
        I i = this;     // cast(void*)*vthis + N
        // But N is not known at compile time, so we have to add a new parameter to __I_foo_require.
    }
}

class C : I
{
    override void foo(/*C vthis, */int x)
    in  // __C_foo_require
    {
    }
    body
    {
        // in here, &x == &vthis + (void*).sizeof
        try {
            // When I fixed issue 9383, I've changed to pass a temporary address &__tmp
            // as the context pointer of nexted function __I_foo_require.
            // I __tmp = this;
            // By that, 'this' access inside __I_foo_require was fixed, but it has introduced 15984.

            // To fix 15984, we have to pass &vthis to __I_foo_require,
            // otherwise it cannot access to member function parameter 'x'
            __I_foo_require(N);
        }
        catch { __C_foo_require(); }

    }
}

// ---

interface J { ... }

class D : J, I
{
    override void foo(int x)
    in { ... }
    body { /* Of course, the runtime offset D to I is different from the C to I. */ }
}

[rainers]

FYI: when porting the original fix to LDC, I chose to just pass the adjusted this pointer to the interface contracts. No magic required to change the actual pointer in ThisExp.
With this, your test case runs further than when built with DMD 2.071, but still asserts in C.f3.i.

[wilzbach]

PRs pointing at stable shouldn't get stalled. It should be dead simple: either it's a regression fix, then merge or target master instead. I will look into implementing a warning at dlang-bot for all pulls to stable which are older than three days.

[wilzbach]

Closing and opening to hopefully remove this from auto-tester's priority list:

[ibuclaw]

PR that introduced the regression was #4788

[thewilsonator]

Rebased as #8988