Skip to content

Fix scoping and slice update in Array.reserve#22351

Merged
dkorpel merged 1 commit intodlang:masterfrom
gorsing:fixroot_array_unused_value
Jan 7, 2026
Merged

Fix scoping and slice update in Array.reserve#22351
dkorpel merged 1 commit intodlang:masterfrom
gorsing:fixroot_array_unused_value

Conversation

@gorsing
Copy link
Contributor

@gorsing gorsing commented Jan 6, 2026

This MR fixes a correctness issue in Array.reserve(), where the slice was rebound
using a pointer declared in a narrower scope.

The code now explicitly updates data to point to the newly allocated or
reallocated storage in each branch, ensuring correct variable lifetime and
well-defined behavior.

There is no behavioral change:

  • debug(stomp) still uses allocate–copy–stomp–free
  • the non-debug path still relies on xrealloc

This change improves correctness and robustness without affecting semantics.

#22327
#22284

@dlang-bot
Copy link
Contributor

Thanks for your pull request and interest in making D better, @gorsing! 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 + dmd#22351"

@gorsing gorsing changed the title fixroot_array Fix scoping and slice update in Array.reserve Jan 6, 2026
@dkorpel
Copy link
Contributor

dkorpel commented Jan 6, 2026

I don't understand the justification; the common line of code was factored out of the two version branches, now it's duplicated again. How does this improve 'correctness and robustness'?

@gorsing
Copy link
Contributor Author

gorsing commented Jan 6, 2026

@dkorpel Thanks for your time
The common line assumed a pointer lifetime that did not clearly extend across both branches.
Updating the slice within each branch makes the lifetime explicit and helps avoid any reliance on configuration-dependent or ill-defined behavior.

@dkorpel
Copy link
Contributor

dkorpel commented Jan 6, 2026

version(X) {}, debug {}, and static if {} don't introduce new scopes. If they did, you'd get an error "undefined identifier p". D doesn't allow using variables that have gone out of scope.

@gorsing
Copy link
Contributor Author

gorsing commented Jan 6, 2026

Thanks — you’re right that version, debug, and static if don’t introduce new scopes.

The reason I kept the slice update inside each branch is to make the storage’s lifetime and ownership explicit: in debug(stomp) the storage is freed, while in the non-debug path it is preserved via xrealloc. Keeping the update local clarifies this distinction without changing behavior.

@dkorpel
Copy link
Contributor

dkorpel commented Jan 7, 2026

I don't think this is worth discussing at length so I'll just merge this, but in the future please make code changes based on actual issues from dmd usage, instead of perceived (lifetime) issues that are actually not causing any problems at all.

@dkorpel dkorpel merged commit 1dea637 into dlang:master Jan 7, 2026
41 of 42 checks passed
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.

4 participants