Skip to content
This repository has been archived by the owner on May 20, 2022. It is now read-only.

Tag type and macro #47

Merged
merged 4 commits into from Oct 6, 2021
Merged

Conversation

cmyr
Copy link
Collaborator

@cmyr cmyr commented Oct 5, 2021

Okay so I ended up just going and doing this with a proc macro.

I was reluctant to go this route because generally adding a proc macro for something this minor is bad, because normally you don't already have proc macros, and so you're bringing in a bunch of new dependencies, and it can significantly impact compile time. Since we're already building otspec_macros, however, adding one more doesn't feel that bad.

So this adds a Tag type (struct Tag([u8; 4])) and a tag! macro, that takes a &str literal, and verifies at compile time that it is a valid tag.

  • 1-4 characters long (we add padding spaces if it's shorter, so tag!("URD") == b"URD ".
  • all bytes in the range 0x20..=0x7E

If either of those things is false, then code won't compile.

The main advantage to this approach is that you can print a tag and know it's valid utf-8, and that all tags are generated statically at compile time. (If you need to create one at runtime you can do Tag::from_raw("hi").unwrap()).

Overall I think this is... okay? it works, and is clear. I'm annoyed that it ended up being so much code, especially since I would like to make it much easier to grab commonly used tables, which would let me delete a lot of these changes.

One note: there's currently a weird thing where the macro is re-exported from otspec::types, which is itself reexported at fonttools root. This means that if you use fonttools::types::!("hi") it will be a compiler error if otspec is not in your Cargo.toml. I'm not sure what the best solution is for this (probably just only exporting the macro via fonttools?) but I'm going to leave it as-is for now, because I want to go for a walk before the sun goes down.

cheers!

@cmyr
Copy link
Collaborator Author

cmyr commented Oct 6, 2021

I can fix this up now

This would definitely be overkill if we did not already have some
procedural macros, but as-is it's a nice solution that gives us
compile-time guarantees and doesn't cost us much.
@simoncozens
Copy link
Owner

Urgh. I tried, but I don't understand that error.

@cmyr
Copy link
Collaborator Author

cmyr commented Oct 6, 2021

okay I'll take a look :)

@simoncozens
Copy link
Owner

I'm inclined to merge this for now, but I'm weighing up trading off compile-time checking for API friendliness. Almost every call to get_table is now going to look like get_table(tag!(...)), which makes me want to put the tag conversion inside the get_table implementation. And tag! is a nasty internal otspec thing which fonttools users should not have to care about.

@cmyr
Copy link
Collaborator Author

cmyr commented Oct 6, 2021

yes, I more or less agree.

I would like to dig into table access shortly, and have some ideas there that should reduce the need for tag!, will continue in #45.

@simoncozens simoncozens merged commit dca4139 into simoncozens:main Oct 6, 2021
@cmyr cmyr deleted the tag-type-and-macro branch October 6, 2021 14:16
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants