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

rfc: phase-independent functions with ?inflight keyword #1711

Closed
wants to merge 3 commits into from

Conversation

Chriscbr
Copy link
Contributor

@Chriscbr Chriscbr commented Mar 3, 2023

Inspired by a recent post on the Rust blog about how "maybe-async" and "maybe-const" type signatures could be supported in the Rust ecosystem, I thought it could be interesting to consider a syntax for defining phase-independent functions in Wing using the ?inflight keyword. This doc proposes how it would look and how it would work. Related to #474 and #435

From a compiler perspective, these functions would essentially be code generated in both the preflight JS class and the inflight JS class.

Alternatives considered

Let the compiler infer whether a function can be phase-independent

In this model, all functions are phase-independent by default, and capturing a resource would automatically make it a preflight function.

In my opinion, the phase of a function isn't something the compiler should infer -- not because it's not technically feasible, but of user experience problems it could introduce.

In the future, devs will be able to create a Wing libraries that export resources with public preflight methods and public inflight methods. When using a Wing library, consumers should be able to rely on the fact that the method always works in that phase in the future, even if it the implementation changes -- just like today you can rely on the fact that a if a imported TypeScript function returns a boolean, then it will always return a boolean in the future (unless there is a breaking change).

If the compiler inferred whether a function should be phase-independent or preflight, then changing the implementation of the function so that it captures some mutable data could change the function's type to preflight, thus causing a potentially breaking change to consumers.

Make phase-independent functions the default, and add preflight as a function modifier instead

I think the idea of functions being phase-independent as the default could be interesting. But I feel like it might be more confusing since a lot of the Wing language is currently designed around the concept that "the top level of your code is preflight" -- for example, creating a bucket at the top-level scope of a Wing file happens in preflight. And the default constructor of a resource is its preflight constructor, which matches up with that.

Put another way, I think preflight-only functions will be more common than phase-independent functions, so I think there should be more friction for the phase-independent use case than the preflight code use case. But this is definitely something we can experiment with.

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

@Chriscbr Chriscbr requested a review from a team as a code owner March 3, 2023 03:27
@Chriscbr
Copy link
Contributor Author

Chriscbr commented Mar 3, 2023

Another idea/suggestion is to designate these functions with the keyword pure

@hasanaburayyan
Copy link
Collaborator

Im on board, I just dont much care for the prefixed ? maybe a keyword instead like pure but no strong opinions here.

@ShaiBer
Copy link
Contributor

ShaiBer commented Mar 5, 2023

I agree that letting the compiler infer whether a function is phase-independent is not the right way to go.
I also agree that making functions phase-independent should not be the default because it sends the wrong message about which type of function we expect to be more prevalent if writing correct wing code.

Like @hasanaburayyan, I'm also not crazy about the ?inflight keyword. I like Rust's approach with maybe-inflight better, but it also doesn't feel like the best solution to me. I hope someone will come up with a better alternative everyone will like.

I'm also OK with experimenting with having to always put a phase modifier on functions, and then we have preflight, inflight and maybe-inflight (or ?inflight or something better someone else will come up with)

@ekeren
Copy link
Collaborator

ekeren commented Mar 5, 2023

@Chriscbr , great doc
Can you also add the reasoning and need for phase-independent methods, have you came across any concrete use cases?

@marciocadev
Copy link
Collaborator

marciocadev commented Mar 5, 2023

Perhaps an alternative is to create functions that are neither inflight nor preflight, some flight functions and add the prefix when assigning to a variable

This type of function only becomes executable when the prefix is assigned

something like

// a template function, if no variable receives this template function
// with a prefix (pre/in) the compiler gives an error 
let f = flight(...) { ... }; 
let p = pre f; // create a preflight function
let i = in f; // create a inflight function

@ShaiBer
Copy link
Contributor

ShaiBer commented Mar 5, 2023

@Chriscbr , great doc Can you also add the reasoning and need for phase-independent methods, have you came across any concrete use cases?

Maybe I'm missing something, but I think any project has general purpose utility functions that are used throughout the code and have no business being confined to only one phase. For example: sorting an array with different algorithms

@Chriscbr
Copy link
Contributor Author

Chriscbr commented Mar 6, 2023

Yes, I think @ShaiBer 's answer is correct, the main use case that comes to my mind is utility functions that do not need to be restricted to one phase or another. If I think of any more use cases I'll update the proposal.

BTW, my initial guess is we don't need this feature for beta, or at least we should dogfood more before considering it.

@marciocadev that's a very interesting idea! From an implementation angle, I think that could be a good option if we want to simplify modeling phase-independent functions in the compiler (perhaps we would only need our Phase enum to have two choices instead of three).

But I'm a little cautious about templating based solutions / not sure I understand the benefits. In your example, suppose I use a preflight function inside f (this means it can only be used in preflight). Would I only get a compiler error once I write let i = in f? Or would I get an error on the let f = line?

@marciocadev
Copy link
Collaborator

marciocadev commented Mar 6, 2023

I was thinking in gives an error if f is declare and never used to create a inflight or preflight function

Some errors

bring cloud;

let f = flight ( ... ) { ... } // ERROR, something like f variable never used
bring cloud;

let f = flight ( ... ) { ... }

new cloud.Function(pre f); // ERROR, can not pass a preflight to cloud.Function

This works

bring cloud;

let f = flight ( ... ) { ... }

new cloud.Function(in f); // WORKS

Somehow, I think of a preflight function as a function that will only be performed during the stack deploy. It is something that can be executed during the deploy phase, either before building the stack, during its construction, or after it has been built.

bring cloud;

let f = flight ( ... ) { ... } // verify some data
let p = pre f;
if (p.invoke() == false) { // verify before create the cloud.Function (preflight)
   ...
}

let i = in f;

new cloud.Function(inflight(event: str): str => {
   ...
  if (i.invoke() == true) { // verify inside cloud.Function (inflight)
     ...
  }
  ...
}

if (p.invoke() == true) { // verify after create the cloud.Function (preflight)
   ...
}

or the same without create new variables

bring cloud;

let f = flight ( ... ) { ... } // verify some data
if ((pre f).invoke() == false) { // verify before create the cloud.Function (preflight)
   ...
}

new cloud.Function(inflight(event: str): str => {
   ...
  if ((in f).invoke() == true) { // verify inside cloud.Function (inflight)
     ...
  }
  ...
}

if ((pre f).invoke() == true) { // verify after create the cloud.Function (preflight)
   ...
}

@Chriscbr
Copy link
Contributor Author

Another naming suggestion from @MarkMcCulloh is "unphased"

@github-actions
Copy link

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!

@github-actions github-actions bot added the Stale label Apr 18, 2023
@Chriscbr Chriscbr removed the Stale label Apr 18, 2023
@github-actions
Copy link

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!

@github-actions github-actions bot added the Stale label May 11, 2023
@Chriscbr Chriscbr removed the Stale label May 14, 2023
@Chriscbr Chriscbr marked this pull request as draft May 26, 2023 18:45
@github-actions
Copy link

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!

@github-actions github-actions bot added the Stale label Jun 17, 2023
@Chriscbr Chriscbr removed the Stale label Jun 20, 2023
@skyrpex
Copy link
Contributor

skyrpex commented Aug 29, 2023

I'd expect Wing to infer the phase of the function most of the time, and also cast a function to inflight if necessary. In the same way that TypeScript can infer that the following function returns a number:

export const sum = (a: number, b: number) => {
  return a + b;
};

Regarding Chris' point:

When using a Wing library, consumers should be able to rely on the fact that the method always works in that phase in the future, even if it the implementation changes

I like Mark's unphased keyword to enforce that a function works in both preflight and inflight phases.

Here's how I'd like it to work in different scenarios:

Compiler infers that the function is inflight

bring cloud;

new cloud.Function(() => {
  return "hello";
});

Compiler can cast phase-independent functions to inflight (if they don't interact with resources nor capture mutable state)

bring cloud;

let sum = (a: num, b: num) => {
  return a + b;
}

log(sum(1, 1));

new cloud.Function(() => {
  return sum(2, 2);
});

Compiler infers the function is inflight so it has access to bucket's inflight API

bring cloud;

let bucket = new cloud.Bucket();

new cloud.Function(() => {
  bucket.put("hello", "world");
});

Compiler infers that "put" is preflight because it accesses the bucket

bring cloud;

let bucket = new cloud.Bucket();

let putFile = (key: str, value: str) => {
  bucket.put(key, value);
  // ^ Error: "bucket.put" is an inflight method but "putFile" is a preflight method. Try adding the inflight keyword to "putFile".
};

Compiler infers that "append" is preflight because it captures mutable state

bring cloud;

let mut state = "";

let append = (value: str) => {
  state = "${state} ${value}";
};

new cloud.Function(() => {
  append("hello");
  // ^ Error: Can't use "append" because it's a preflight function that captures mutable state.
});

@Chriscbr
Copy link
Contributor Author

@skyrpex Feel free to add a +1 to the issue about inferring function phases here: #3755

The intuition in your examples is correct - I think unphased functions should be possible to use both in places that expect preflight functions and places that expect inflight functions.

@Chriscbr Chriscbr mentioned this pull request Oct 6, 2023
5 tasks
@exoego exoego mentioned this pull request Oct 8, 2023
5 tasks
Copy link

github-actions bot commented May 5, 2024

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!

@github-actions github-actions bot added the Stale label May 5, 2024
@github-actions github-actions bot closed this May 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants