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

Signature of closure - return type should be mandatory (even if it is void) #2234

Closed
eladb opened this issue Apr 24, 2023 · 12 comments · Fixed by #2733
Closed

Signature of closure - return type should be mandatory (even if it is void) #2234

eladb opened this issue Apr 24, 2023 · 12 comments · Fixed by #2733
Assignees
Labels
🐛 bug Something isn't working 🛠️ compiler Compiler good first issue Good for newcomers 📐 language-design Language architecture

Comments

@eladb
Copy link
Contributor

eladb commented Apr 24, 2023

I tried this

Declare a variable for a closure that doesn't return a value (void).

I expected this:

To look like this:

let my_closure: inflight (): void;

Instead, this happened

Apparently this is the syntax:

let my_closure: inflight ();

Is there a workaround?

N/A

Component

Language Design

Wing Version

No response

Wing Console Version

No response

Node.js Version

No response

Platform(s)

No response

Anything else?

No response

Community Notes

  • Please vote by adding a 👍 reaction to the issue to help us prioritize.
  • If you are interested to work on this issue, please leave a comment.
@eladb eladb added the 🐛 bug Something isn't working label Apr 24, 2023
@Chriscbr Chriscbr added 🛠️ compiler Compiler 📐 language-design Language architecture labels Apr 24, 2023
@staycoolcall911 staycoolcall911 changed the title Signature of closure with void return type is very odd Signature of closure - return type is mandatory (even if it is void) Apr 25, 2023
@staycoolcall911
Copy link
Contributor

@yoav-steinberg - this means we will also have to add void #432 to the language

@eladb eladb changed the title Signature of closure - return type is mandatory (even if it is void) Signature of closure - return type should be mandatory (even if it is void) Apr 25, 2023
@yoav-steinberg
Copy link
Collaborator

Hey, what's the reasoning behind this? I think that last language to adopt this approach was C++...

@MarkMcCulloh
Copy link
Contributor

I think that last language to adopt this approach was C++

Typescript works like this

// not valid
let my_closure: ();

// valid
let my_closure: () => void;

@eladb
Copy link
Contributor Author

eladb commented May 3, 2023

The reasoning is ergonomic/aesthetic in my mind. This () doesn't feel like it's a type that represents a function signature in my mind.

@staycoolcall911
Copy link
Contributor

We have decided to move along with Elad's suggested approach and expose void as return type for closures not returning anything. This should also handle #2378

@yoav-steinberg
Copy link
Collaborator

Lets see how it feels to make signatures and closures require void while methods definitions keeping it optional:

let var_of_sig_type: (x: num): void; // Function type annotation requires void
let my_func = (callback: (x:num): void): void => { ... }; // callback argument type annotation requires void, closure requires void
let my_func2 = (x: num): void => { ... }; // again, closure definition requires void
class C {
  my_method(x: num) { ... } // No need for void here
  my_method2(x: num): void { ... } // But also works when return type explicitly annotated as void
}
interface IPointOfView {
  computeTheMeaningOfLifeTheUniverseAndEverything(x: num); // Here too void can be implicit
  computeTheMeaningOfLifeTheUniverseAndEverything2(x: num): void; // .. Or explicit
}

@yoav-steinberg yoav-steinberg added the good first issue Good for newcomers label May 17, 2023
@eladb
Copy link
Contributor Author

eladb commented May 17, 2023

@yoav-steinberg why can't we make void optional in closure definitions?

Callback argument type annotation requires void (because it's a declaration), closure DOES NOT requires void (because it's a definition):

let my_func = (callback: (x:num): void) => { ... }; 

Closure definitions DOES NOT require void:

let my_func2 = (x: num) => { ... };

What do you think?

@yoav-steinberg
Copy link
Collaborator

@yoav-steinberg why can't we make void optional in closure definitions?

Sure, lets do that!

@eladb
Copy link
Contributor Author

eladb commented May 17, 2023

BTW, I think the interface declarations SHOULD HAVE void (because they are declarations):

interface IPointOfView {
  computeTheMeaningOfLifeTheUniverseAndEverything(x: num): void;
  // ...
}

@yoav-steinberg
Copy link
Collaborator

yoav-steinberg commented May 18, 2023

Revised reference given above comments:

let var_of_sig_type: (x: num): void; // Function type annotation requires void
let my_func = (callback: (x:num): void) => { ... }; // callback argument type annotation requires void, closure definition doesn't require void
let my_func2 = (x: num) => { ... }; // again, closure definition doesn't require void
let my_func3 = (x: num): void => { ... }; // but can have it
class C {
  my_method(x: num) { ... } // No need for void here
  my_method2(x: num): void { ... } // But also works when return type explicitly annotated as void
}
interface IPointOfView {
  computeTheMeaningOfLifeTheUniverseAndEverything(x: num): void; // In interfaces we require void, since it's a declaration (type annotation)
}

@eladb
Copy link
Contributor Author

eladb commented May 18, 2023

Ack

@mergify mergify bot closed this as completed in #2733 May 31, 2023
mergify bot pushed a commit that referenced this issue May 31, 2023
This allows using `void` as a builtin type and requires a return type for function types and interface methods
 
Fixes #2234 

## Checklist

- [X] Title matches [Winglang's style guide](https://docs.winglang.io/contributors/pull_requests#how-are-pull-request-titles-formatted)
- [X] Description explains motivation and solution
- [X] Tests added (always)
- [ ] Docs updated (only required for features)
- [ ] Added `pr/e2e-full` label if this feature requires end-to-end testing

*By submitting this pull request, I confirm that my contribution is made under the terms of the [Monada Contribution License](https://docs.winglang.io/terms-and-policies/contribution-license.html)*.
@monadabot
Copy link
Contributor

Congrats! 🚀 This was released in Wing 0.18.1.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🐛 bug Something isn't working 🛠️ compiler Compiler good first issue Good for newcomers 📐 language-design Language architecture
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

7 participants