mark freeze_expr function as unsafe

[kitcatier]

gluon/vm/src/core/mod.rs

Lines 602 to 614 in 27cec6c

	pub fn freeze_expr<'a>(allocator: &'a Arc<Allocator<'a>>, expr: CExpr<'a>) -> CoreExpr {
	unsafe {
	// Here we temporarily forget the lifetime of `allocator` so it can be moved into a
	// `CoreExpr`. After we have it in `CoreExpr` the expression is then guaranteed to live as
	// long as the `CoreExpr` making it safe again
	CoreExpr::new(
	FrozenAllocator(mem::transmute::<Arc<Allocator>, Arc<Allocator<'static>>>(
	allocator.clone(),
	)),
	mem::transmute::<CExpr, CExpr<'static>>(expr),
	)
	}
	}

gluon/vm/src/core/mod.rs

Lines 616 to 635 in 27cec6c

	pub fn freeze_closure<'a>(
	allocator: &'a Arc<Allocator<'a>>,
	id: &'a TypedIdent<Symbol>,
	args: &'a [TypedIdent<Symbol>],
	expr: CExpr<'a>,
	) -> CoreClosure {
	unsafe {
	// Here we temporarily forget the lifetime of `allocator` so it can be moved into a
	// `CoreClosure`. After we have it in `CoreClusre` the expression is then guaranteed to live as
	// long as the `CoreClosure` making it safe again
	CoreClosure::new(
	FrozenAllocator(mem::transmute::<Arc<Allocator>, Arc<Allocator<'static>>>(
	allocator.clone(),
	)),
	mem::transmute::<&TypedIdent<Symbol>, &TypedIdent<Symbol>>(id),
	mem::transmute::<&[TypedIdent<Symbol>], &[TypedIdent<Symbol>]>(args),
	mem::transmute::<CExpr, CExpr<'static>>(expr),
	)
	}
	}

Hi, it is not a good choice to mark the entire function body as unsafe, which will make the caller ignore the safety requirements that the function parameters must guarantee, the developer who calls the freeze_expr function may not notice this safety requirement.

And inside the function body, mem::transmute is used twice, which is an unsafe operation. The first time is to convert the type of allocator.clone() from Arc to Arc<Allocator<'static>>, and the second time is to convert expr of type CExpr to type CExpr<'static>. These operations will make the type system unable to guarantee the type safety of the code, so it is necessary to use the unsafe keyword to mark the function and remind users to use it with caution.

Marking them unsafe also means that callers must make sure they know what they're doing.

[Marwes]

These functions should be safe to the best of my knowledge. The idea that I tried to explain with the comment is that the reference to the expression is guaranteed to be from the arena allocator (enforced by the lifetime annotations), using that knowledge we can return a new value without any lifetimes since the allocator which the reference lives in will live at least as long as the reference itself, thus it is impossible to do any use-after-free with it.

If you have a counter example that would allow that, please let me know.