Skip to content

Conversation

@kinke
Copy link
Member

@kinke kinke commented Jun 29, 2016

Don't perform a vtable lookup for those, resolve them directly instead.
Fixes issue #1450, where calling the base method in the overriding method resulted in an infinite loop and corresponding stack overflow.

Don't perform a vtable lookup for those, resolve them directly instead.
Fixes issue ldc-developers#1450, where calling the base method in the overwritten
method resulted in an infinite loop and corresponding stack overflow.
@kinke
Copy link
Member Author

kinke commented Jun 29, 2016

We have the same check for DelegateExp, the only other place where we call DtoVirtualFunctionPointer().

Test will follow once merged. ;)

@kinke
Copy link
Member Author

kinke commented Jun 29, 2016

Updated as I figured the actual reason, not just the symptom. This was just begging for more issues. See my monologue in #1450. ;)

@kinke
Copy link
Member Author

kinke commented Jun 29, 2016

I'm planning to extend the direct-sret-construction to return statements as well, not just variable declarations.
Currently, NRVO produces ideal IR for something like auto r = super.foo(); return r; - the sret pointer is simply forwarded to super.foo().
Returning another return value directly (i.e., return super.foo();) currently performs an obsolete copy and possibly even a postblit call.

@JohanEngelen
Copy link
Member

Test will follow once merged. ;)

For dmd-testsuite additions, I understand. But for IR tests, could you add them to the PR? (or perhaps it doesn't involve IR tests yet)

@kinke
Copy link
Member Author

kinke commented Jun 30, 2016

There's no need to check the IR for this. It's actually quite hard to test this (properly visiting the CallExp); the testcase in #1450 can test one (current) symptom if it isn't visited.

@dnadlinger dnadlinger merged commit 72f1c21 into ldc-developers:master Jun 30, 2016
@dnadlinger
Copy link
Member

Seems like Travis was having network connectivity issues again; hope I didn't miss any failures because of that.

@kinke
Copy link
Member Author

kinke commented Jun 30, 2016

@kinke kinke deleted the super branch July 1, 2016 17:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants