-
Notifications
You must be signed in to change notification settings - Fork 657
feat(rome_js_syntax): better api for picking jsx attributes #3458
Conversation
✅ Deploy Preview for rometools canceled.
|
Parser conformance results on ubuntu-latestjs/262
jsx/babel
symbols/microsoft
ts/babel
ts/microsoft
|
0a373bb
to
81ab611
Compare
) -> [Option<JsxAttribute>; N] { | ||
// assert there are no duplicates | ||
debug_assert!(HashSet::<_>::from_iter(names_to_lookup).len() == N); | ||
debug_assert!(N <= 16); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why is it necessary that N
is smaller than 16?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just to put a safeguard, given that everything is allocated on the stack.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would you mind add a comment to explain why? I personally don't know why that is safe and against what. That would be appreciated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done.
7fceed3
to
9967517
Compare
9967517
to
28c86f4
Compare
Summary
This PR try to simplify the API for retrieving JSX attributes.
Unfortunately, to achieve this it needs to use unsafe, because
JsxAttribute
is notCopy
.Test Plan