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

Doc RawTableInner::find_insert_slot and also make it unsafe #405

Merged
merged 3 commits into from
Feb 23, 2023

Conversation

JustForFun88
Copy link
Contributor

Made this function unsafe, as it is possible to use the index returned by this function unsafely. Here are two functions that confirm the documentation I made:

#[test]
fn test_infinite_loop() {
	use ::alloc::vec::Vec;
	let mut table: RawTable<u16> = RawTable::with_capacity(20);

	assert_eq!(table.capacity(), 28);
	assert_eq!(table.buckets(), 32);
	for i in 0..table.buckets() {
		unsafe {
			let bucket_index = table.table.find_insert_slot(i as u64);

			table.table.set_ctrl_h2(bucket_index, i as u64);
			table.table.items += 1;

			table.bucket(bucket_index).write(i as u16);
		}
	}

	let vec = unsafe { table.iter().map(|x| x.read()).collect::<Vec<_>>() };

	assert_eq!(vec, [0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19, 20, 21, 22, 23, 24, 25, 26, 27, 28, 29, 30, 31]);

	// this never return
	let _bucket_index = unsafe { table.table.find_insert_slot(32) };
}

#[test]
fn test_undefined_behavior() {
	use ::alloc::vec::Vec;

	let mut table: RawTable<u16> = RawTable::with_capacity(3);

	assert_eq!(table.capacity(), 3);
	assert_eq!(table.buckets(), 4);
	for i in 0..3 {
		table.insert(i, i as u16, |i| *i as u64);
	}

	assert_eq!(table.table.items, 3);
	assert_eq!(table.table.growth_left, 0);

	let bucket_index = unsafe { table.table.find_insert_slot(3_u64) };

	assert_eq!(bucket_index, 3);

	unsafe {
		table.table.set_ctrl_h2(bucket_index, 3_u64);
		table.table.items += 1;
		table.bucket(bucket_index).write(3_u16);
	}

	let vec = unsafe { table.iter().map(|x| x.read()).collect::<Vec<_>>() };

	assert_eq!(vec, [0, 1, 2, 3]);
	let bucket_index = unsafe { table.table.find_insert_slot(4_u64) };
	assert_eq!(bucket_index, 4);
}

@JustForFun88 JustForFun88 marked this pull request as ready for review February 23, 2023 09:38
@Amanieu
Copy link
Member

Amanieu commented Feb 23, 2023

This function should not be unsafe. As you said in the doc, it is always safe to call. unsafe should only be used when a function can cause UB with certain arguments.

@JustForFun88
Copy link
Contributor Author

This function should not be unsafe. As you said in the doc, it is always safe to call. unsafe should only be used when a function can cause UB with certain arguments.

I removed unsafe and changed "Safety" to "Note"

@Amanieu
Copy link
Member

Amanieu commented Feb 23, 2023

@bors r+

@bors
Copy link
Contributor

bors commented Feb 23, 2023

📌 Commit 1605442 has been approved by Amanieu

It is now in the queue for this repository.

@bors
Copy link
Contributor

bors commented Feb 23, 2023

⌛ Testing commit 1605442 with merge d40ffa1...

@bors
Copy link
Contributor

bors commented Feb 23, 2023

☀️ Test successful - checks-actions
Approved by: Amanieu
Pushing d40ffa1 to master...

@bors bors merged commit d40ffa1 into rust-lang:master Feb 23, 2023
@JustForFun88 JustForFun88 deleted the find_insert_slot branch February 23, 2023 12:59
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