Skip to content

another correction for caller destroying args#16167

Merged
RazvanN7 merged 1 commit into
dlang:masterfrom
WalterBright:callerDestroy2
Mar 6, 2024
Merged

another correction for caller destroying args#16167
RazvanN7 merged 1 commit into
dlang:masterfrom
WalterBright:callerDestroy2

Conversation

@WalterBright
Copy link
Copy Markdown
Member

This is a fix for this problem:

#16145 (comment)

In case that PR never gets pulled. Don't know how to trigger the test case without the rest of that PR.

The problem is that functions that return a static array with elements that need to be destructed should not be inlined, because the e2ir.d code no longer will call _d_arrayctor() to do it.

@dlang-bot
Copy link
Copy Markdown
Contributor

Thanks for your pull request, @WalterBright!

Bugzilla references

Your PR doesn't reference any Bugzilla issue.

If your PR contains non-trivial changes, please reference a Bugzilla issue or create a manual changelog.

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#16167"

@WalterBright WalterBright added the Review:Ready To Merge Let's get this merged label Feb 13, 2024
@WalterBright
Copy link
Copy Markdown
Member Author

This is ready to merge.

@thewilsonator
Copy link
Copy Markdown
Contributor

does this fix anything? Why does it not have a test suite change?

@WalterBright
Copy link
Copy Markdown
Member Author

It's a partial fix for #16145 . If we don't go with 16415, this fix is still important in case of a change of mind in the future. But it cannot be tested without implementing 16145.

I.e. the code promises that caller/callee destruction is set by a flag in target.d. It should keep its promise!

@ibuclaw
Copy link
Copy Markdown
Member

ibuclaw commented Feb 16, 2024

There's a PR for removing the frontend inliner. #16173

@RazvanN7 RazvanN7 merged commit f957472 into dlang:master Mar 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Review:Easy Review Review:Ready To Merge Let's get this merged Severity:Refactoring No semantic changes to code

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants