Skip to content

Comments

Add return scope to realloc#8116

Merged
dlang-bot merged 1 commit intodlang:masterfrom
dkorpel:realloc-return-scope
Jun 8, 2021
Merged

Add return scope to realloc#8116
dlang-bot merged 1 commit intodlang:masterfrom
dkorpel:realloc-return-scope

Conversation

@dkorpel
Copy link
Contributor

@dkorpel dkorpel commented May 29, 2021

Split off from #8113
Since realloc could return the input if there is enough capacity, it should be return scope. While in practice realloc is all @system and @trusted code, it does seem to trip up issue 20150.

@dlang-bot
Copy link
Contributor

Thanks for your pull request and interest in making D better, @dkorpel! We are looking forward to reviewing it, and you should be hearing from a maintainer soon.
Please verify that your PR follows this checklist:

  • My PR is fully covered with tests (you can see the coverage diff by visiting the details link of the codecov check)
  • My PR is as minimal as possible (smaller, focused PRs are easier to review than big ones)
  • I have provided a detailed rationale explaining my changes
  • New or modified functions have Ddoc comments (with Params: and Returns:)

Please see CONTRIBUTING.md for more information.


If you have addressed all reviews or aren't sure how to proceed, don't hesitate to ping us with a simple comment.

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 + phobos#8116"

@dkorpel dkorpel force-pushed the realloc-return-scope branch from 77b1841 to c6f2cbf Compare May 29, 2021 12:29
@dkorpel
Copy link
Contributor Author

dkorpel commented May 29, 2021

Turns out that std.internal.cstring: tempCString is tricky, since it dynamically switches between stack and heap memory, which trips up scope.

@dkorpel dkorpel marked this pull request as draft June 3, 2021 19:37
@dkorpel dkorpel force-pushed the realloc-return-scope branch 3 times, most recently from af02c31 to 569b8c5 Compare June 3, 2021 21:17
@dkorpel dkorpel marked this pull request as ready for review June 3, 2021 23:24
@dkorpel
Copy link
Contributor Author

dkorpel commented Jun 3, 2021

Turns out that std.internal.cstring: tempCString is tricky, since it dynamically switches between stack and heap memory, which trips up scope.

I now split up the stack-logic and heap-logic into two functions, so that the former can be scope while the latter return scope.

@RazvanN7
Copy link
Collaborator

RazvanN7 commented Jun 7, 2021

Please rebase to restart circleCI

@wilzbach
Copy link
Contributor

wilzbach commented Jun 7, 2021

@RazvanN7 maybe you could rebase for the author? The ping, rebase, wait for auto-merge label to be re-added cycle seems a bit inefficient.

@RazvanN7
Copy link
Collaborator

RazvanN7 commented Jun 8, 2021

@wilzbach Of course I can do the rebase myself, however, I think it's better to keep the PR author engaged.

@RazvanN7 RazvanN7 force-pushed the realloc-return-scope branch from 02dc46b to 9c932d3 Compare June 8, 2021 08:45
@dlang-bot dlang-bot merged commit 5242beb into dlang:master Jun 8, 2021
@dkorpel dkorpel deleted the realloc-return-scope branch June 8, 2021 11:28
@dkorpel
Copy link
Contributor Author

dkorpel commented Jun 8, 2021

@wilzbach Of course I can do the rebase myself, however, I think it's better to keep the PR author engaged.

Having to rebase my PRs over and over again because of CI issues is not very engaging though 😬

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants