-
-
Notifications
You must be signed in to change notification settings - Fork 892
Allow spreading records in type constructors for const values #5061
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
base: main
Are you sure you want to change the base?
Conversation
b2242d8 to
200b09a
Compare
50c471a to
df799f4
Compare
df799f4 to
5d39a71
Compare
lpil
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you! This will be a relly nice addition. I've left a few questions in line.
Could you add a test that ensures that pub const dog = Animal(..alice) emits the "fieldless record update" warning please 🙏
84302ed to
5f48b6d
Compare
7f5b0cd to
3ca3271
Compare
lpil
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you! Looking fab. I've left some notes inline, mostly around behaviour that doesn't match record updates for expressions.
f8e45cb to
faaa1e5
Compare
lpil
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Left a note inline!
...c/type_/tests/snapshots/gleam_core__type___tests__errors__expression_constructor_update.snap
Outdated
Show resolved
Hide resolved
faaa1e5 to
c6bb035
Compare
9326063 to
e6a00d9
Compare
|
I suppose I now need to accommodate for changes in c2fed7e! |
ed56e68 to
4f7ba1a
Compare
lpil
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you make the error messages for constants exactly the same as expressions please 🙏
Could we not return the same error value for constants as expressions? Then they would be rendered the same to the user.
|
@lpil Absolutely, I can work on this! |
1dd8f75 to
57778f2
Compare
a0f539f to
3ceec04
Compare
|
@lpil Dependency validation fails and this is not directly related to the PR 🤔
|
```gleam const a = Foo(1, 2) const b = Foo(..a, 3) ``` Code like this now compiles, which makes constant construction a lot simpler!
Accommodate for recent changes to error handling in const inference in c2fed7e
Remove unused insta snapshots including ones left after renaming const record update tests.
Refactor the record update logic so const updates use the same error types as regular record updates. This produces consistent messages for users and removes cases where similar problems surfaced in different forms. The change: - restores InvalidRecordConstructor instead of InvalidRecordUpdate with reasons - uses UnsafeRecordUpdate for variant mismatches to align with expression handling - adds a constructor_location field so the compiler can point at the constructor when reporting InvalidRecordConstructor - and introduces validation for incompatible field types in spreads
3ceec04 to
5a89ff1
Compare
Uh oh!
There was an error while loading. Please reload this page.