Skip to content

Commit 9651fc1

Browse files
nicopapnicopap
committed
Merge Access and AccessRef using a Cow
mooooo
1 parent b100069 commit 9651fc1

File tree

2 files changed

+73
-89
lines changed

2 files changed

+73
-89
lines changed

‎crates/bevy_reflect/src/path/access.rs

Lines changed: 48 additions & 74 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
use std::fmt;
1+
use std::{borrow::Cow, fmt};
22

33
use super::{AccessError, ReflectPathError};
44
use crate::{Reflect, ReflectMut, ReflectRef, VariantType};
@@ -13,10 +13,7 @@ pub(super) enum Error<'a> {
1313
access.kind(),
1414
access.display_value(),
1515
)]
16-
Access {
17-
ty: TypeShape,
18-
access: AccessRef<'a>,
19-
},
16+
Access { ty: TypeShape, access: Access<'a> },
2017

2118
#[error("invalid type shape: expected {expected} but found a reflect {actual}")]
2219
Type {
@@ -36,6 +33,10 @@ impl<'a> Error<'a> {
3633
let error = AccessError(self);
3734
ReflectPathError::InvalidAccess { offset, error }
3835
}
36+
37+
fn access(ty: TypeShape, access: Access<'a>) -> Self {
38+
Self::Access { ty, access }
39+
}
3940
}
4041
impl Error<'static> {
4142
fn bad_enum_variant(expected: TypeShape, actual: impl Into<TypeShape>) -> Self {
@@ -101,20 +102,18 @@ impl From<VariantType> for TypeShape {
101102
}
102103
}
103104

104-
/// A singular owned element access within a path.
105+
/// A singular element access within a path.
105106
///
106107
/// Can be applied to a `dyn Reflect` to get a reference to the targeted element.
107-
///
108-
/// A path is composed of multiple accesses in sequence.
109-
#[derive(Debug, Clone, PartialEq, Eq, Hash, PartialOrd, Ord)]
110-
pub(super) enum Access {
111-
Field(Box<str>),
108+
#[derive(Debug, Clone, PartialEq, Eq, PartialOrd, Ord, Hash)]
109+
pub(super) enum Access<'a> {
110+
Field(Cow<'a, str>),
112111
FieldIndex(usize),
113112
TupleIndex(usize),
114113
ListIndex(usize),
115114
}
116115

117-
impl fmt::Display for Access {
116+
impl fmt::Display for Access<'_> {
118117
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
119118
match self {
120119
Access::Field(field) => write!(f, ".{field}"),
@@ -125,35 +124,10 @@ impl fmt::Display for Access {
125124
}
126125
}
127126

128-
impl Access {
129-
pub(super) fn as_ref(&self) -> AccessRef<'_> {
130-
match self {
131-
Self::Field(value) => AccessRef::Field(value),
132-
Self::FieldIndex(value) => AccessRef::FieldIndex(*value),
133-
Self::TupleIndex(value) => AccessRef::TupleIndex(*value),
134-
Self::ListIndex(value) => AccessRef::ListIndex(*value),
135-
}
136-
}
137-
}
138-
139-
/// A singular borrowed element access within a path.
140-
///
141-
/// Can be applied to a `dyn Reflect` to get a reference to the targeted element.
142-
///
143-
/// Does not own the backing store it's sourced from.
144-
/// For an owned version, you can convert one to an [`Access`] with [`AccessRef::to_owned`].
145-
#[derive(Debug, Clone, Copy, PartialEq, Eq)]
146-
pub(super) enum AccessRef<'a> {
147-
Field(&'a str),
148-
FieldIndex(usize),
149-
TupleIndex(usize),
150-
ListIndex(usize),
151-
}
152-
153-
impl<'a> AccessRef<'a> {
154-
pub(super) fn to_owned(self) -> Access {
127+
impl<'a> Access<'a> {
128+
pub(super) fn into_owned(self) -> Access<'static> {
155129
match self {
156-
Self::Field(value) => Access::Field(value.to_string().into_boxed_str()),
130+
Self::Field(value) => Access::Field(value.to_string().into()),
157131
Self::FieldIndex(value) => Access::FieldIndex(value),
158132
Self::TupleIndex(value) => Access::TupleIndex(value),
159133
Self::ListIndex(value) => Access::ListIndex(value),
@@ -174,78 +148,78 @@ impl<'a> AccessRef<'a> {
174148
}
175149
}
176150

177-
pub(super) fn element(
178-
self,
179-
base: &dyn Reflect,
151+
pub(super) fn element<'r>(
152+
&self,
153+
base: &'r dyn Reflect,
180154
offset: usize,
181-
) -> Result<&dyn Reflect, ReflectPathError<'a>> {
155+
) -> Result<&'r dyn Reflect, ReflectPathError<'a>> {
182156
let ty = base.reflect_ref().into();
183157
self.element_inner(base)
184-
.and_then(|maybe| maybe.ok_or(Error::Access { ty, access: self }))
158+
.and_then(|maybe| maybe.ok_or(Error::access(ty, self.clone())))
185159
.map_err(|err| err.with_offset(offset))
186160
}
187161

188-
fn element_inner(self, base: &dyn Reflect) -> InnerResult<&dyn Reflect> {
162+
fn element_inner<'r>(&self, base: &'r dyn Reflect) -> InnerResult<&'r dyn Reflect> {
189163
use ReflectRef::*;
190164
match (self, base.reflect_ref()) {
191-
(Self::Field(field), Struct(struct_ref)) => Ok(struct_ref.field(field)),
165+
(Self::Field(field), Struct(struct_ref)) => Ok(struct_ref.field(field.as_ref())),
192166
(Self::Field(field), Enum(enum_ref)) => match enum_ref.variant_type() {
193-
VariantType::Struct => Ok(enum_ref.field(field)),
167+
VariantType::Struct => Ok(enum_ref.field(field.as_ref())),
194168
actual => Err(Error::bad_enum_variant(TypeShape::Struct, actual)),
195169
},
196-
(Self::FieldIndex(index), Struct(struct_ref)) => Ok(struct_ref.field_at(index)),
197-
(Self::FieldIndex(index), Enum(enum_ref)) => match enum_ref.variant_type() {
170+
(&Self::FieldIndex(index), Struct(struct_ref)) => Ok(struct_ref.field_at(index)),
171+
(&Self::FieldIndex(index), Enum(enum_ref)) => match enum_ref.variant_type() {
198172
VariantType::Struct => Ok(enum_ref.field_at(index)),
199173
actual => Err(Error::bad_enum_variant(TypeShape::Struct, actual)),
200174
},
201-
(Self::TupleIndex(index), TupleStruct(tuple)) => Ok(tuple.field(index)),
202-
(Self::TupleIndex(index), Tuple(tuple)) => Ok(tuple.field(index)),
203-
(Self::TupleIndex(index), Enum(enum_ref)) => match enum_ref.variant_type() {
175+
(&Self::TupleIndex(index), TupleStruct(tuple)) => Ok(tuple.field(index)),
176+
(&Self::TupleIndex(index), Tuple(tuple)) => Ok(tuple.field(index)),
177+
(&Self::TupleIndex(index), Enum(enum_ref)) => match enum_ref.variant_type() {
204178
VariantType::Tuple => Ok(enum_ref.field_at(index)),
205179
actual => Err(Error::bad_enum_variant(TypeShape::Tuple, actual)),
206180
},
207-
(Self::ListIndex(index), List(list)) => Ok(list.get(index)),
208-
(Self::ListIndex(index), Array(list)) => Ok(list.get(index)),
209-
(Self::ListIndex(_), actual) => Err(Error::bad_type(TypeShape::List, actual)),
181+
(&Self::ListIndex(index), List(list)) => Ok(list.get(index)),
182+
(&Self::ListIndex(index), Array(list)) => Ok(list.get(index)),
183+
(&Self::ListIndex(_), actual) => Err(Error::bad_type(TypeShape::List, actual)),
210184
(_, actual) => Err(Error::bad_type(TypeShape::Struct, actual)),
211185
}
212186
}
213187

214-
pub(super) fn element_mut(
215-
self,
216-
base: &mut dyn Reflect,
188+
pub(super) fn element_mut<'r>(
189+
&self,
190+
base: &'r mut dyn Reflect,
217191
offset: usize,
218-
) -> Result<&mut dyn Reflect, ReflectPathError<'a>> {
192+
) -> Result<&'r mut dyn Reflect, ReflectPathError<'a>> {
219193
let ty = base.reflect_ref().into();
220194
self.element_inner_mut(base)
221-
.and_then(|maybe| maybe.ok_or(Error::Access { ty, access: self }))
195+
.and_then(|maybe| maybe.ok_or(Error::access(ty, self.clone())))
222196
.map_err(|err| err.with_offset(offset))
223197
}
224198

225-
fn element_inner_mut(self, base: &mut dyn Reflect) -> InnerResult<&mut dyn Reflect> {
199+
fn element_inner_mut<'r>(&self, base: &'r mut dyn Reflect) -> InnerResult<&'r mut dyn Reflect> {
226200
use ReflectMut::*;
227-
let base_kind: TypeShape = base.reflect_ref().into();
201+
let base_shape: TypeShape = base.reflect_ref().into();
228202
match (self, base.reflect_mut()) {
229-
(Self::Field(field), Struct(struct_mut)) => Ok(struct_mut.field_mut(field)),
203+
(Self::Field(field), Struct(struct_mut)) => Ok(struct_mut.field_mut(field.as_ref())),
230204
(Self::Field(field), Enum(enum_mut)) => match enum_mut.variant_type() {
231-
VariantType::Struct => Ok(enum_mut.field_mut(field)),
205+
VariantType::Struct => Ok(enum_mut.field_mut(field.as_ref())),
232206
actual => Err(Error::bad_enum_variant(TypeShape::Struct, actual)),
233207
},
234-
(Self::FieldIndex(index), Struct(struct_mut)) => Ok(struct_mut.field_at_mut(index)),
235-
(Self::FieldIndex(index), Enum(enum_mut)) => match enum_mut.variant_type() {
208+
(&Self::FieldIndex(index), Struct(struct_mut)) => Ok(struct_mut.field_at_mut(index)),
209+
(&Self::FieldIndex(index), Enum(enum_mut)) => match enum_mut.variant_type() {
236210
VariantType::Struct => Ok(enum_mut.field_at_mut(index)),
237211
actual => Err(Error::bad_enum_variant(TypeShape::Struct, actual)),
238212
},
239-
(Self::TupleIndex(index), TupleStruct(tuple)) => Ok(tuple.field_mut(index)),
240-
(Self::TupleIndex(index), Tuple(tuple)) => Ok(tuple.field_mut(index)),
241-
(Self::TupleIndex(index), Enum(enum_mut)) => match enum_mut.variant_type() {
213+
(&Self::TupleIndex(index), TupleStruct(tuple)) => Ok(tuple.field_mut(index)),
214+
(&Self::TupleIndex(index), Tuple(tuple)) => Ok(tuple.field_mut(index)),
215+
(&Self::TupleIndex(index), Enum(enum_mut)) => match enum_mut.variant_type() {
242216
VariantType::Tuple => Ok(enum_mut.field_at_mut(index)),
243217
actual => Err(Error::bad_enum_variant(TypeShape::Tuple, actual)),
244218
},
245-
(Self::ListIndex(index), List(list)) => Ok(list.get_mut(index)),
246-
(Self::ListIndex(index), Array(list)) => Ok(list.get_mut(index)),
247-
(Self::ListIndex(_), _) => Err(Error::bad_type(TypeShape::List, base_kind)),
248-
(_, _) => Err(Error::bad_type(TypeShape::Struct, base_kind)),
219+
(&Self::ListIndex(index), List(list)) => Ok(list.get_mut(index)),
220+
(&Self::ListIndex(index), Array(list)) => Ok(list.get_mut(index)),
221+
(&Self::ListIndex(_), _) => Err(Error::bad_type(TypeShape::List, base_shape)),
222+
(_, _) => Err(Error::bad_type(TypeShape::Struct, base_shape)),
249223
}
250224
}
251225
}

‎crates/bevy_reflect/src/path/mod.rs

Lines changed: 25 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ use std::fmt;
44
use std::num::ParseIntError;
55

66
use crate::Reflect;
7-
use access::{Access, AccessRef};
7+
use access::Access;
88
use thiserror::Error;
99

1010
type ParseResult<T> = Result<T, ReflectPathParseError>;
@@ -291,7 +291,7 @@ pub struct ParsedPath(
291291
/// index of the start of the access within the parsed path string.
292292
///
293293
/// The index is mainly used for more helpful error reporting.
294-
Box<[(Access, usize)]>,
294+
Box<[(Access<'static>, usize)]>,
295295
);
296296

297297
impl ParsedPath {
@@ -343,7 +343,17 @@ impl ParsedPath {
343343
pub fn parse(string: &str) -> Result<Self, ReflectPathError<'_>> {
344344
let mut parts = Vec::new();
345345
for (access, idx) in PathParser::new(string) {
346-
parts.push((access?.to_owned(), idx));
346+
parts.push((access?.into_owned(), idx));
347+
}
348+
Ok(Self(parts.into_boxed_slice()))
349+
}
350+
351+
/// Similar to [`Self::parse`] but only works on `&'static str`
352+
/// and does not allocate per named field.
353+
pub fn parse_static(string: &'static str) -> Result<Self, ReflectPathError<'_>> {
354+
let mut parts = Vec::new();
355+
for (access, idx) in PathParser::new(string) {
356+
parts.push((access?, idx));
347357
}
348358
Ok(Self(parts.into_boxed_slice()))
349359
}
@@ -359,7 +369,7 @@ impl ParsedPath {
359369
) -> Result<&'r dyn Reflect, ReflectPathError<'p>> {
360370
let mut current = root;
361371
for (access, current_index) in self.0.iter() {
362-
current = access.as_ref().element(current, *current_index)?;
372+
current = access.element(current, *current_index)?;
363373
}
364374
Ok(current)
365375
}
@@ -375,7 +385,7 @@ impl ParsedPath {
375385
) -> Result<&'r mut dyn Reflect, ReflectPathError<'p>> {
376386
let mut current = root;
377387
for (access, current_index) in self.0.iter() {
378-
current = access.as_ref().element_mut(current, *current_index)?;
388+
current = access.element_mut(current, *current_index)?;
379389
}
380390
Ok(current)
381391
}
@@ -471,15 +481,15 @@ impl<'a> PathParser<'a> {
471481
Some(ident)
472482
}
473483

474-
fn token_to_access(&mut self, token: Token<'a>) -> ParseResult<AccessRef<'a>> {
484+
fn token_to_access(&mut self, token: Token<'a>) -> ParseResult<Access<'a>> {
475485
let current_offset = self.index;
476486
match token {
477487
Token::Dot => {
478488
if let Some(Token::Ident(value)) = self.next_token() {
479489
value
480490
.parse::<usize>()
481-
.map(AccessRef::TupleIndex)
482-
.or(Ok(AccessRef::Field(value)))
491+
.map(Access::TupleIndex)
492+
.or(Ok(Access::Field(value.into())))
483493
} else {
484494
Err(ReflectPathParseError::ExpectedIdent {
485495
offset: current_offset,
@@ -488,7 +498,7 @@ impl<'a> PathParser<'a> {
488498
}
489499
Token::CrossHatch => {
490500
if let Some(Token::Ident(value)) = self.next_token() {
491-
Ok(AccessRef::FieldIndex(value.parse::<usize>()?))
501+
Ok(Access::FieldIndex(value.parse::<usize>()?))
492502
} else {
493503
Err(ReflectPathParseError::ExpectedIdent {
494504
offset: current_offset,
@@ -497,7 +507,7 @@ impl<'a> PathParser<'a> {
497507
}
498508
Token::OpenBracket => {
499509
let access = if let Some(Token::Ident(value)) = self.next_token() {
500-
AccessRef::ListIndex(value.parse::<usize>()?)
510+
Access::ListIndex(value.parse::<usize>()?)
501511
} else {
502512
return Err(ReflectPathParseError::ExpectedIdent {
503513
offset: current_offset,
@@ -519,14 +529,14 @@ impl<'a> PathParser<'a> {
519529
}),
520530
Token::Ident(value) => value
521531
.parse::<usize>()
522-
.map(AccessRef::TupleIndex)
523-
.or(Ok(AccessRef::Field(value))),
532+
.map(Access::TupleIndex)
533+
.or(Ok(Access::Field(value.into()))),
524534
}
525535
}
526536
}
527537

528538
impl<'a> Iterator for PathParser<'a> {
529-
type Item = (ParseResult<AccessRef<'a>>, usize);
539+
type Item = (ParseResult<Access<'a>>, usize);
530540

531541
fn next(&mut self) -> Option<Self::Item> {
532542
let token = self.next_token()?;
@@ -615,7 +625,7 @@ mod tests {
615625
}
616626

617627
fn access_field(field: &'static str) -> Access {
618-
Access::Field(field.to_string().into_boxed_str())
628+
Access::Field(field.into())
619629
}
620630

621631
#[test]
@@ -778,7 +788,7 @@ mod tests {
778788
offset: 2,
779789
error: AccessError(access::Error::Access {
780790
ty: TypeShape::Struct,
781-
access: AccessRef::Field("notreal"),
791+
access: access_field("notreal"),
782792
}),
783793
}
784794
);

0 commit comments

Comments
 (0)