add static id / paths construction #1860
Conversation
|
|
||
| /// This is how we can create a `Name` at compile time | ||
| #[macro_export] | ||
| macro_rules! make_name { |
There was a problem hiding this comment.
@john-h-kastner-aws this is sort of how I think we should allow for compile time name creation.
There was a problem hiding this comment.
looks like a reasonable approach to me. It should be trivial to have a wrapper around all this in the cedar-policy crate to construct the public Name type, right?
|
|
||
| #[test] | ||
| fn compile_time_name() { | ||
| const _: Name = make_name!("foo").expect("should be OK"); |
There was a problem hiding this comment.
IIUC, this expect is also evaluated at compile time, correct?
There was a problem hiding this comment.
yes thats correct, this is not a std::result::Result, so this is calling a different method i defined called expect which is const.
|
|
||
| /// Similar to `new_unchecked`, but for static strings which can be `const` | ||
| /// constructed | ||
| pub(crate) const fn new_unchecked_from_static(s: &'static str) -> Id { |
There was a problem hiding this comment.
This is kind of nice change on its own. Wouldn't mind splitting it out into a separate small PR
| } | ||
|
|
||
| /// Check if the `Id` is static | ||
| pub const fn is_static(&self) -> bool { |
There was a problem hiding this comment.
Would it be better to use the name is_heap_allocated instead since this function will return true for an inline smolstr?
| } | ||
| } | ||
|
|
||
| const fn validate_part(part: &str) -> Option<NameValidationError> { |
There was a problem hiding this comment.
My main concern for merging a final implementation will be the correctness of this function. Ideally some of the logic should be shared (at least the list of reserved identifiers), and then unit tests ofc, but we can also do something relatively lightweight with the proptest crate
There was a problem hiding this comment.
I agree, it would be ideal if we could share the logic. Not sure how to go about that tho.
|
|
||
| /// This is how we can create a `Name` at compile time | ||
| #[macro_export] | ||
| macro_rules! make_name { |
There was a problem hiding this comment.
looks like a reasonable approach to me. It should be trivial to have a wrapper around all this in the cedar-policy crate to construct the public Name type, right?
2 participants
Description of changes
Allow for construction of IDs, and entity type names at compile time.
Issue #, if available
#1828
Checklist for requesting a review
The change in this PR is (choose one, and delete the other options):
cedar-policy(e.g., changes to the signature of an existing API).cedar-policy(e.g., addition of a new API).cedar-policy.cedar-policy-core,cedar-validator, etc.)I confirm that this PR (choose one, and delete the other options):
I confirm that
cedar-spec(choose one, and delete the other options):cedar-spec, and how you have tested that your updates are correct.)cedar-spec. (Post your PR anyways, and we'll discuss in the comments.)I confirm that
docs.cedarpolicy.com(choose one, and delete the other options):cedar-docs. PRs should be targeted at astaging-X.Ybranch, notmain.)