winglang / wing

A programming language for the cloud ☁️ A unified programming model, combining infrastructure and runtime code into one language ⚡
https://winglang.io
Other
4.68k stars 181 forks source link

fix(compiler): resource without an initializer #1861

Closed marciocadev closed 1 year ago

marciocadev commented 1 year ago

Closes #1705

By submitting this pull request, I confirm that my contribution is made under the terms of the Monada Contribution License.

revitalbarletz commented 1 year ago

@yoav-steinberg can you please review?

eladb commented 1 year ago

@marciocadev seems like you need to re-request a review from @Chriscbr as well

Chriscbr commented 1 year ago

Another way of viewing this feature is "starting in x.y.z, Wing implicitly provides a default constructor when you don't write one". For now it will only work in basic cases (when the resource has zero instance members, and when there is no parent class) but we can make it more powerful in the future.

Chriscbr commented 1 year ago

@marciocadev I opened up a draft pull request to your PR here to show you what I am thinking of with leaving the constructor as a required AST field - let me know if the direction makes sense.

marciocadev commented 1 year ago

Now, when creating a class with fields but without an initializer, no error is generated, which is problematic because it should be reported that the field was not initialized.

I will need to validate the initialization of the field in the constructor.

example

class T {
   x: num;
}
Chriscbr commented 1 year ago

I see... maybe we need some kind of static analysis here? pinging @yoav-steinberg

marciocadev commented 1 year ago

I see... maybe we need some kind of static analysis here? pinging @yoav-steinberg

@Chriscbr I fixed this, but I'm not sure if it's the correct way to do it.

@yoav-steinberg, could you please check if this is correct?

I listed the assignments inside the initializer, and if one of the class fields is missing from the list, I raise an error that points to the missing field.

yoav-steinberg commented 1 year ago

I see... maybe we need some kind of static analysis here? pinging @yoav-steinberg

@Chriscbr I fixed this, but I'm not sure if it's the correct way to do it.

@yoav-steinberg, could you please check if this is correct?

I listed the assignments inside the initializer, and if one of the class fields is missing from the list, I raise an error that points to the missing field.

Just making sure there's an assignment for each fields isn't good enough. We need some static analysis to get this right, and even then I'm not sure we can catch all cases. I was thinking that for now we just ignore this and hope the user doesn't leave any field uninitialized. The same goes for making sure super is called when we have a parent.

Supporting a resource/class without an initializer should only be valid for classes with no fields and no parent.

github-actions[bot] commented 1 year ago

Hi,

This PR has not seen activity in 20 days. Therefore, we are marking the PR as stale for now. It will be closed after 7 days. If you need help with the PR, do not hesitate to reach out in the winglang community slack at winglang.slack.com. Feel free to re-open this PR when it is still relevant and ready to be worked on again. Thanks!

eladb commented 1 year ago

@yoav-steinberg any updates on this?

marciocadev commented 1 year ago

Guys, this branch is very outdated and I think it's better to create a new branch with this implementation. I will ask @Chriscbr, @yoav-steinberg, and @eladb to review the new branch

I'm going to close this branch.

staycoolcall911 commented 1 year ago

@marciocadev - sorry this one got left behind. We’ll work with you to get the next branch on the fast lane. Thank you for your patience.

marciocadev commented 1 year ago

Don't worry @staycoolcall911

I have some health issues last month so I can't work on that

staycoolcall911 commented 1 year ago

Oh, hoping you're feeling better now, @marciocadev !

marciocadev commented 1 year ago

I'm fine now, back to work 😁

staycoolcall911 commented 1 year ago

👏