Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

BTreeMap uses Global allocator in it's PhantomData even when instantiated with custom Allocator #121797

Closed
shamatar opened this issue Feb 29, 2024 · 5 comments · Fixed by #121847
Labels
A-allocators Area: Custom and system allocators A-collections Area: std::collections. C-bug Category: This is a bug. T-libs Relevant to the library team, which will review and decide on the PR/issue.

Comments

@shamatar
Copy link
Contributor

I'm referring to this particular line

_marker: PhantomData<crate::boxed::Box<(K, V)>>,

Even though in no_std environments Global as a structure exists and is defined, and it only shows up in PhantomData, the use of the structure is most likely undefined if #[global_allocator] attribute is never used. Yes, there will be no calls to Global as it's only a phantom, and execution will not be affected, but just seems ill-defined at least.

For completeness: compilation of code that uses alloc::collection::BTreeMap with custom allocator under #![feature(btreemap_alloc)] is not affected, and there is no sign of runtime bugs.

@shamatar shamatar added the C-bug Category: This is a bug. label Feb 29, 2024
@rustbot rustbot added the needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. label Feb 29, 2024
@jieyouxu jieyouxu added the T-libs Relevant to the library team, which will review and decide on the PR/issue. label Feb 29, 2024
@ChayimFriedman2
Copy link
Contributor

Does this matter? It holds it directly:

pub(super) alloc: ManuallyDrop<A>,

@shamatar
Copy link
Contributor Author

My issue is the opposite - it should be

_marker: PhantomData<crate::boxed::Box<(K, V), A>>,

in the PhantomData too, but not implicit

_marker: PhantomData<crate::boxed::Box<(K, V), Global>>,

@ChayimFriedman2
Copy link
Contributor

I understand, but it already includes A for dropck, so why should it include it in the PhantomData too?

@shamatar
Copy link
Contributor Author

To avoid inclusion of Global that has no logical relation to the BTreeMap instance with custom allocator

@shamatar
Copy link
Contributor Author

If it's accepted as a bug I can quickly fix it, at least it passes tests

@fmease fmease added A-allocators Area: Custom and system allocators A-collections Area: std::collections. and removed needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. labels Mar 1, 2024
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Mar 2, 2024
Dylan-DPC added a commit to Dylan-DPC/rust that referenced this issue Mar 2, 2024
@bors bors closed this as completed in 78249a6 Mar 2, 2024
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Mar 2, 2024
rust-timer added a commit to rust-lang-ci/rust that referenced this issue Mar 2, 2024
Rollup merge of rust-lang#121847 - shamatar:btreemap_fix_implicits, r=cuviper

Remove hidden use of Global

Fixes rust-lang#121797
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-allocators Area: Custom and system allocators A-collections Area: std::collections. C-bug Category: This is a bug. T-libs Relevant to the library team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants