taiki-e / pin-project-lite

A lightweight version of pin-project written with declarative macros.
https://docs.rs/pin-project-lite
Apache License 2.0
216 stars 15 forks source link

[WIP] refactors visibility parsing #65

Closed Michael-J-Ward closed 3 years ago

Michael-J-Ward commented 3 years ago

Working towards handling newtype-variants on enums, slimming down what will need to change for that.

Before, visibility parsing / downgrade logic was implemented separately for structs and enums. Since this was done at the same stage as lifetime / field / variant parsing, the result was a decent amount of duplicated code.

Now, visibility is parsed first before dispatching to the separate struct/enum parsing.

Michael-J-Ward commented 3 years ago

I see there are some ui tests that break.

The next step was to refactor out a generics parsing step, so if you approve of the code change, can create a PR into this branch to accumulate the update, and then handle the UI stuff at the end?

Michael-J-Ward commented 3 years ago

@taiki-e so I see how to run the UI tests, I've made a few changes to try and match them.

The latest commit removed the @generics directive to improve the compiler error diagnostics. pin_project_lite's readme says explicitly "No useful error messages", so I'm unsure how much effort I should put into the UI errors, especially since they are likely to change again as I continue working towards enabling newtype variants, additional attributes on fields and const generics.

taiki-e commented 3 years ago

UI tests actually work as tests to check that the problematic code is rejected correctly and track changes in error messages in long term. (especially after #36) So, there isn't much need to work on the error message at this time. (Actually, some of the ERROR annotations in the current main branch are also wrong.)

Having said that, the change you made in https://github.com/taiki-e/pin-project-lite/pull/65/commits/a2ce9aaca814e0021573c4b96080b426d054a8dd seems to make sense.

(FWIW, originally, "No useful error messages" meant things like "don't have arms for error handling" and "don't treat error message regressions as blockers".)

Michael-J-Ward commented 3 years ago

This work superseded by #69