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

Implement Type Aliases #1163

Open
wants to merge 61 commits into
base: canary
Choose a base branch
from
Open

Implement Type Aliases #1163

wants to merge 61 commits into from

Conversation

antoniosarosi
Copy link
Contributor

@antoniosarosi antoniosarosi commented Nov 12, 2024

Implementation of TS-like type aliases (structural recursion allowed through maps and lists).

Basic aliases:

type A = int
type B = float
type C = A | B

Recursive aliases:

type RecursiveMap = map<string, RecursiveMap>

type A = A[]

type JsonValue = int | float | bool | string | null | JsonValue[] | map<string, JsonValue> 

See also #1207

Copy link

vercel bot commented Nov 12, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
baml ✅ Ready (Inspect) Visit Preview 💬 Add feedback Dec 18, 2024 8:28pm

@antoniosarosi antoniosarosi changed the title Add grammar for type aliases Implement Type Aliases Nov 12, 2024
@antoniosarosi antoniosarosi marked this pull request as ready for review December 9, 2024 16:57
Copy link
Contributor

ellipsis-dev bot commented Dec 9, 2024

⚠️ This PR is too big for Ellipsis, but support for larger PRs is coming soon. If you want us to prioritize this feature, let us know at [email protected]


Generated with ❤️ by ellipsis.dev

@imalsogreg
Copy link
Contributor

imalsogreg commented Dec 10, 2024

Something strange going on in the playground. These two errors only appear once I add a type alias. The name I chose, Foo, is of course maybe problematic (if it overlaps with some other Foo in our big baml_src). But I see the same behavior when I use a unique name for the type alias.

Screenshot 2024-12-09 at 6 32 35 PM

@antoniosarosi
Copy link
Contributor Author

@imalsogreg Fixed!

<!-- ELLIPSIS_HIDDEN -->


> [!IMPORTANT]
> Adds `RunFoo2` function across clients, new types, refines recursive
type handling, and updates unreachable code with context.
> 
>   - **New Functionality**:
> - Adds `RunFoo2` function to Python (`async_client.py`,
`sync_client.py`), Ruby (`client.rb`), and TypeScript
(`async_client.ts`, `sync_client.ts`) clients.
> - Introduces `Foo2` and `Foo3` types in `types.py`, `types.rb`, and
`types.ts`.
>   - **Code Improvements**:
> - Replaces `unreachable!()` with `unreachable!("context")` in
`expr.rs`, `coerce_array.rs`, `coerce_optional.rs`, and
`coerce_union.rs`.
>     - Refactors recursive type handling in `types.rs`.
>   - **Testing**:
>     - Adds `test_constrained_type_alias` in `test_runtime.rs`.
>     - Adds `test_type_alias_with_assert` in `test_file_manager.rs`.
>     - Adds `test_alias_bug` in `test_functions.py`.
>   - **Miscellaneous**:
>     - Adds `antonio.baml` test file for integration tests.
> 
> <sup>This description was created by </sup>[<img alt="Ellipsis"
src="https://img.shields.io/badge/Ellipsis-blue?color=175173">](https://www.ellipsis.dev?ref=BoundaryML%2Fbaml&utm_source=github&utm_medium=referral)<sup>
for 10f4999. It will automatically
update as commits are pushed.</sup>


<!-- ELLIPSIS_HIDDEN -->

---------

Co-authored-by: Greg Hale <[email protected]>
@imalsogreg
Copy link
Contributor

This is just a thought for later, definitely not something i would suggest implementing now:

Some time in the future, I think FieldType::Alias could be removed from the FieldType definition, and type aliases should be a form of syntactic sugar, rather than a core part of the IR.

Pros/Cons to this proposal:

  • Pro: FieldType::Alias isn't necessary in the IR because a type alias is always just a user-facing name for a type, not a semantically distinct concept. (When type Foo = int, the compiler doesn't distinguish between the two - the difference is only present at a surface syntax/UI level).
  • Pro: Having two representations for the same entity causes duplicate code for many non-type-alias related calculations. I.e. many IR operations need to account for FieldType::Alias and all its intricacies, when in fact at this compiler stage only the resolved field is relevant.
  • Con: FieldType::Alias is already implemented and well understood, we would be changing it when it's not broken, under this proposal.
  • Con: Maybe it's not obvious which compiler pass should be responsible for desugaring type aliases.
  • Con: We would need to carry metadata (maybe on IR nodes, maybe on types) to track the syntax provenance of a type, in order to use the correct spelling (raw type spelling or alias spelling) in different contexts. It might not be obvious where we should store that metadata. However, FieldType probably deserves the same FieldTypeWithMeta<T> treatment that we did for BamlValue.

Again - this is just some thought about the future, definitely not a change we should make now. But I'm curious @antoniosarosi what you think about it.

@antoniosarosi
Copy link
Contributor Author

antoniosarosi commented Dec 16, 2024

Thought about removing FieldType::Alias from the IR yesterday, the only reason I added that was to report errors using either the alias name or target/resolved type depending on the context. For this particular PR it could be removed but for the cycles PR it's still necessary to store the alias name somewhere. Still thinking about how to refactor it @imalsogreg.

Allow structural recursion (like TS):

```baml
type RecursiveMap = map<string, RecursiveMap>

type A = A[]
```

Follow up on #1163

<!-- ELLIPSIS_HIDDEN -->


----

> [!IMPORTANT]
> Enable structural recursion in type aliases by updating cycle
validation logic, adding tests, and supporting recursive type aliases in
Python, Ruby, and TypeScript clients.
> 
>   - **Behavior**:
> - Allow structural recursion for type alias cycles involving maps and
lists in `cycle.rs`.
> - Update `validate()` in `cycle.rs` to handle structural recursion and
report cycles.
> - Modify `ParserDatabase::finalize_dependencies()` in `lib.rs` to
resolve type aliases and handle cycles.
>   - **Functions**:
> - Add `insert_required_alias_deps()` in `cycle.rs` to handle alias
dependencies.
> - Rename `insert_required_deps()` to `insert_required_class_deps()` in
`cycle.rs`.
>   - **Models**:
>     - Add `structural_recursive_alias_cycles` to `Types` in `mod.rs`.
>   - **Tests**:
> - Update `recursive_type_aliases.baml` to reflect new cycle validation
behavior.
>     - Add tests in `lib.rs` to verify structural recursion handling.
>   - **Language Support**:
> - Update Python, Ruby, and TypeScript clients to handle recursive type
aliases.
>   - **Misc**:
> - Add `is_subtype()` to `IRHelper` in `mod.rs` to support subtyping
checks.
> - Add `coerce_alias()` in `coerce_alias.rs` to handle alias coercion.
> - Add integration tests for recursive alias cycles in
`integ-tests/typescript/tests/integ-tests.test.ts`.
> 
> <sup>This description was created by </sup>[<img alt="Ellipsis"
src="https://img.shields.io/badge/Ellipsis-blue?color=175173">](https://www.ellipsis.dev?ref=BoundaryML%2Fbaml&utm_source=github&utm_medium=referral)<sup>
for fc25050. It will automatically
update as commits are pushed.</sup>


<!-- ELLIPSIS_HIDDEN -->
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