Commit Graph

5 Commits

Author SHA1 Message Date
Félix Lescaudey de Maneville
a2b5d7a198
Fix some nightly warnings (#9672)
# Objective

Fix some nightly warnings found by running

`cargo +nightly clippy`

## Solution

Fix the following warnings:
- [x]
[elided_lifetimes_in_associated_constant](https://github.com/rust-lang/rust/issues/115010)
221986134d
- [x]
[unwrap_or_default](https://rust-lang.github.io/rust-clippy/master/index.html#/unwrap_or_default)
32e21c78f9
- [x]
[needless_pass_by_ref_mut](https://rust-lang.github.io/rust-clippy/master/index.html#/needless_pass_by_ref_mut)
c85d6d4a10

There is no breaking change, some internal `bevy_ecs` code no longer
uses a few mutable references but I don't think it should cause any
regression or be performance sensitive, but there might be some ECS
magic I'm unaware of that could break because of those changes
2023-09-02 18:35:06 +00:00
Nicola Papale
365cf3114a
Make the reflect path parser utf-8-unaware (#9371)
# Objective

All delimiter symbols used by the path parser are ASCII, this means we
can entirely ignore UTF8 handling. This may improve performance.

## Solution

Instead of storing the path as an `&str` + the parser offset, and
reading the path using `&self.path[self.offset..]`, we store the parser
state in a `&[u8]`. This allows two optimizations:

1. Avoid UTF8 checking on `&self.path[self.offset..]`
2. Avoid any kind of bound checking, since the length of what is left to
read is stored in the `&[u8]`'s reference metadata, and is assumed valid
by the compiler.

This is a major improvement when comparing to the previous parser.

1. `access_following` and `next_token` now inline in `PathParser::next`
2. Benchmarking show a 20% performance increase (#9364)

Please note that while we ignore UTF-8 handling, **utf-8 is still
supported**. This is because we only handle "at the edges" what happens
exactly before and after a recognized `SYMBOL`. utf-8 is handled
transparently beyond that.
2023-08-25 23:12:25 +00:00
Nicola Papale
7083f0d593
Make it so ParsedPath can be passed to GetPath (#9373)
# Objective

- Unify the `ParsedPath` and `GetPath` APIs. They weirdly didn't play
well together.
- Make `ParsedPath` and `GetPath` API easier to use

## Solution

- Add the `ReflectPath` trait.
- `GetPath` methods now accept an `impl ReflectPath<'a>` instead of a
`&'a str`, this mean it also can accepts a `&ParsedPath`
- Make `GetPath: Reflect` and use default impl for `Reflect` types.
- Add `GetPath` and `ReflectPath` to the `bevy_reflect` prelude

---

## Changelog

- Add the `ReflectPath` trait.
- `GetPath` methods now accept an `impl ReflectPath<'a>` instead of a
`&'a str`, this mean it also can accept a `&ParsedPath`
- Make `GetPath: Reflect` and use default impl for `Reflect` types.
- Add `GetPath` and `ReflectPath` to the `bevy_reflect` prelude

## Migration Guide

`GetPath` now requires `Reflect`. This reduces a lot of boilerplate on
bevy's side. If you were implementing manually `GetPath` on your own
type, please get in touch!

`ParsedPath::element[_mut]` isn't an inherent method of `ParsedPath`,
you must now import `ReflectPath`. This is only relevant if you weren't
importing the bevy prelude.

```diff
-use bevy::reflect::ParsedPath;
+use bevy::reflect::{ParsedPath, ReflectPath};

 parsed_path.element(reflect_type).unwrap()
 parsed_path.element(reflect_type).unwrap()
2023-08-25 14:32:41 +00:00
Nicola Papale
10797d4f15
Refactor parsing in bevy_reflect path module (#9048)
# Objective

- Follow up to #8887
- The parsing code in `bevy_reflect/src/path/mod.rs` could also do with
some cleanup

## Solution

- Create the `parse.rs` module, move all parsing code to this module
- The parsing errors also now keep track of the whole parsed string, and
are much more fine-grained

### Detailed changes

- Move `PathParser` to `parse.rs` submodule
- Rename `token_to_access` to `access_following` (yep, goes from 132
lines to 16)
- Move parsing tests into the `parse.rs` file
2023-08-05 17:18:13 +00:00
Nicola Papale
08ea1d18ae
Refactor path module of bevy_reflect (#8887)
# Objective

- The `path` module was getting fairly large.
- The code in `AccessRef::read_element` and mut equivalent was very
complex and difficult to understand.
- The `ReflectPathError` had a lot of variants, and was difficult to
read.

## Solution

- Split the file in two, `access` now has its own module
- Rewrite the `read_element` methods, they were ~200 lines long, they
are now ~70 lines long — I didn't change any of the logic. It's really
just the same code, but error handling is separated.
- Split the `ReflectPathError` error
- Merge `AccessRef` and `Access`
- A few other changes that aim to reduce code complexity

### Fully detailed change list

- `Display` impl of `ParsedPath` now includes prefix dots — this allows
simplifying its implementation, and IMO `.path.to.field` is a better way
to express a "path" than `path.to.field` which could suggest we are
reading the `to` field of a variable named `path`
- Add a test to check that dot prefixes and other are correctly parsed —
Until now, no test contained a prefixing dot
- Merge `Access` and `AccessRef`, using a `Cow<'a, str>`. Generated code
seems to agree with this decision (`ParsedPath::parse` sheds 5% of
instructions)
- Remove `Access::as_ref` since there is no such thing as an `AccessRef`
anymore.
- Rename `AccessRef::to_owned` into `AccessRef::into_owned()` since it
takes ownership of `self` now.
- Add a `parse_static` that doesn't allocate new strings for named
fields!
- Add a section about path reflection in the `bevy_reflect` crate root
doc — I saw a few people that weren't aware of path reflection, so I
thought it was pertinent to add it to the root doc
- a lot of nits
- rename `index` to `offset` when it refers to offset in the path string
— There is no more confusion with the other kind of indices in this
context, also it's a common naming convention for parsing.
  - Make a dedicated enum for parsing errors
- rename the `read_element` methods to `element` — shorter, but also
`read_element_mut` was a fairly poor name
- The error values now not only contain the expected type but also the
actual type.
- Remove lifetimes that could be inferred from the `GetPath` trait
methods.

---

## Change log

- Added the `ParsedPath::parse_static` method, avoids allocating when
parsing `&'static str`.

## Migration Guide

If you were matching on the `Err(ReflectPathError)` value returned by
`GetPath` and `ParsedPath` methods, now only the parse-related errors
and the offset are publicly accessible. You can always use the
`fmt::Display` to get a clear error message, but if you need
programmatic access to the error types, please open an issue.

---------

Co-authored-by: Alice Cecile <alice.i.cecile@gmail.com>
Co-authored-by: Gino Valente <49806985+MrGVSV@users.noreply.github.com>
2023-07-30 19:17:07 +00:00