Reduce allocation of strings for error messages

[cdisselkoen]

Describe the improvement you'd like to request

Audit error and help messages, particularly those declared with #[error("...")] and #[diagnostic(help("..."))], for those that contain calls to functions that produce String only to incorporate those strings into the main error/help message with {}. For example, in coreschema.rs, the call to pretty_print() in

#[error("context `{}` is not valid for `{action}`", pretty_print(&.context))]

or, also in coreschema.rs, the call to invalid_resource_type_help() in

#[diagnostic(help("{}", invalid_resource_type_help(&.valid_resource_tys, .action.as_ref())))]

These can be replaced with the new thiserror syntax #[error(fmt = some::function::call)] which does not allocate an intermediate String. And presumably some equivalent miette syntax for the diagnostic(help()) case.

This issue is low priority because performance improvements on the error path are low priority, and this code only runs when an error has occurred. It's not even clear that these are the most important performance improvements to be had on the error path. However, they may be reasonably low-hanging fruit.

Describe alternatives you've considered

The status quo is acceptable, particularly because performance improvements on the error path are low priority.

Additional context

No response

Is this something that you'd be interested in working on?

• 👋 I may be able to implement this internal improvement
• ⚠️ This feature might incur a breaking change

[john-h-kastner-aws]

see

cedar/cedar-policy-core/src/ast/policy.rs

Lines 305 to 313 in df4f53e

	/// An error with the slot arguments provided
	// INVARIANT: `unbound_values` and `extra_values` can't both be empty
	#[error(fmt = describe_arity_error)]
	ArityError {
	/// Error for when some Slots were not provided values
	unbound_values: Vec<SlotId>,
	/// Error for when more values than Slots are provided
	extra_values: Vec<SlotId>,
	},

and

cedar/cedar-policy-core/src/ast/policy.rs

Lines 342 to 355 in df4f53e

	fn describe_arity_error(
	unbound_values: &[SlotId],
	extra_values: &[SlotId],
	fmt: &mut std::fmt::Formatter<'_>,
	) -> std::fmt::Result {
	match (unbound_values.len(), extra_values.len()) {
	// PANIC SAFETY 0,0 case is not an error
	#[allow(clippy::unreachable)]
	(0,0) => unreachable!(),
	(_unbound, 0) => write!(fmt, "the following slots were not provided as arguments: {}", unbound_values.iter().join(",")),
	(0, _extra) => write!(fmt, "the following slots were provided as arguments, but did not exist in the template: {}", extra_values.iter().join(",")),
	(_unbound, _extra) => write!(fmt, "the following slots were not provided as arguments: {}. The following slots were provided as arguments, but did not exist in the template: {}", unbound_values.iter().join(","), extra_values.iter().join(",")),
	}
	}

for one error variant that already does this