refactor builtin types in binder#1821
Conversation
|
| /// node kind and its contents. | ||
|
|
||
| #[derive(Clone, Debug)] | ||
| pub enum Typing { |
There was a problem hiding this comment.
@ggiraldez I see that InternalBuiltin is still leaked via Typing and Resolution. Is this intended? should they be moved to use Builtin, given that conversion is cheap?
There was a problem hiding this comment.
We use Typing and Resolution in the AST crate, and that's where it should be contained. It should definitely not reach the public API. It should also be possible to make both types crate pub. I'll explore that and send a PR.
| /// node kind and its contents. | ||
|
|
||
| #[derive(Clone, Debug)] | ||
| pub enum Typing { |
There was a problem hiding this comment.
We use Typing and Resolution in the AST crate, and that's where it should be contained. It should definitely not reach the public API. It should also be possible to make both types crate pub. I'll explore that and send a PR.
OmarTawfik
force-pushed
the
OmarTawfik/stacks/versions/2/rename-builtins-type
branch
from
338c538 to
9517308
Compare
Small rename for the public API: - renames the public `PublicBuiltin` type to just `Builtin`, as it is used in the public API, and the "Public" prefix is redundant. - renames the internal `Builtin` type to `InternalBuiltin`, to make it clear that this should not be leaked to the public API.
OmarTawfik
force-pushed
the
OmarTawfik/stacks/versions/2/rename-builtins-type
branch
from
9517308 to
9e66593
Compare
|
Bencher Report| Branch | OmarTawfik/stacks/versions/2/rename-builtins-type |
| Testbed | ci |
⚠️ WARNING: Truncated view!The full continuous benchmarking report exceeds the maximum length allowed on this platform.
⚠️ WARNING: No Threshold found!Without a Threshold, no Alerts will ever be generated.
🚨 6 Alerts
🐰 View full continuous benchmarking report in Bencher2 participants
Small rename for the public API:
PublicBuiltintype to justBuiltin, as it is used in the public API, and the "Public" prefix is redundant.Builtintype toInternalBuiltin, to make it clear that this should not be leaked to the public API.