Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Migrate metadata fields out of individual elements #3200

Merged
merged 3 commits into from
Jan 16, 2024
Merged

Conversation

laurmaedje
Copy link
Member

@laurmaedje laurmaedje commented Jan 15, 2024

This PR migrates content's metadata fields from being stored in each element individually and accessed via a trait object to being stored directly in the Content itself. This is only an intermediate step in merging the Content and Value types.

Motivation

Currently, Typst has two separate kinds of "types". Proper types and element functions. Instances of the former are Values while instances of the latter are Content. Unifying both would simplify and streamline the language and unlock new capabilities like writing show rules for proper types (datetime, float, etc.). Moreover, it means that we can realize custom elements and custom types through the same mechanism in the future.

To be able to merge Content and Value, Values need to become interoperable with the styling system, which requires the ability to attach a span, a label, a location and some other metadata to it. Currently, this metadata is stored in each kind of element manually (these fields are generated by the #[elem] macro). This approach is incompatible with existing values (we wouldn't want to and really couldn't add all these fields to other kinds of values like integers, floats, or colors). Instead, the metadata should live in the Content / Value itself. This also unlocks optimizations for moving some of the rare metadata into a separate, optional allocation.

However, sometimes we need to interact with an element of a fixed type while still retaining access to its metadata. Currently &self of a HeadingElem provides access to the span. This isn't the case anymore if the span is stored in the Content rather than the HeadingElem. To fix this, this PR introduces a new Packed<T> wrapper that encapsulates an element of well-known type alongside its metadata.

Design

  • A new Packed<T> type is introduced that represents an element that is stored as Content, but is of a known type
  • Packed<T> is a repr(transparent) newtype around Content that derefs to T
  • The Content -> T casts return Option<&Packed<T>>, Option<&mut Packed<T>> and Result<Packed<T>, Content>
  • Packed<T> derefs to T. Here we use some unsafe to skip the check that is already encoded in the type system.
  • Traits like Show, Layout etc. are now implemented on Packed<T> instead of T so that they retain access to span, location, etc.

Notes

  • As said before, we need unsafe code to implement Packed<T>'s deref impl. We wouldn't need this unsafe if Packed<T> had a different representation than Content which held the reference to T directly. However, that would reduce the flexibility of Packed<T> since Content couldn't be coerced into it. We would most probably need three variants of Packed for shared ref, mutable ref, and owned T. And even then a deref of a PackedRef<'a, T> to &T couldn't properly encode the 'a lifetime. Overall, it would be a much greater hassle and the simple repr(transparent) cast isn't that bad.
  • Show for Packed<T> can only be implemented in the crate where Packed is defined because of the orphan rules and the lack of #[fundamental] on stable. This means it would not be possible to implement a new element outside of the typst crate. Still, if that ability should be needed in the future, we could define a decl-macro that makes it simple to define a crate-local Packed alternative in a downstream crate and add an associated Packed type to NativeElement which specifies which Packed type an element uses. Generic uses of Packed<T> would need to be switched to <T as NativeElement>::Packed (which could also be realized through Packed being a type alias to that).
  • This PR removes a bunch of dead code related to mutating fields. This code was unused and probably also mostly untested. In the interest of making the Value rework as simple as possible, I have removed it. We can add it / a modified version back should we introduce more support for mutable fields in the future.

Performance

Some quick benchmarks show that performance is mostly unaffected by this change. However, as mentioned before, this unlocks the capability to do some future optimizations to the merged Value / Content representations like:

  • Storing small values / elements inline
  • Storing label, location, etc. in a separate location

Commits

The first commit contains the mechanical translation of the show rules, while the second one contains the interesting changes.

@laurmaedje laurmaedje added this pull request to the merge queue Jan 16, 2024
Merged via the queue into main with commit c2dfbd3 Jan 16, 2024
8 checks passed
@laurmaedje laurmaedje deleted the packed-content branch January 16, 2024 09:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants