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

Design Meeting Notes, 5/31/2024 #58763

Open
DanielRosenwasser opened this issue Jun 3, 2024 · 3 comments
Open

Design Meeting Notes, 5/31/2024 #58763

DanielRosenwasser opened this issue Jun 3, 2024 · 3 comments
Labels
Design Notes Notes from our design meetings

Comments

@DanielRosenwasser
Copy link
Member

DanielRosenwasser commented Jun 3, 2024

Control Flow Analysis From Callbacks

#11498

let x: string | number = "okay";

mystery(() => {

});

// Nope! TS thinks this is impossible.
if (x === 10) {
}
  • We could assume that the callback to mystery might have been called.
  • Have a prototype that does just this.
    • Basically a new control flow node inserted when a call expression contains a function expressions.
    • When one is found, the body of the lambda is considered a possible branch of the control flow graph for the following code.
  • Rather than adding a modifier for every call, it's arguable that the correct thing here is to be conservative and assume that the callback might have been called.
  • We don't seem to have tests that are sensitive to this pattern, but it is worth running with as an experiment.
  • People often think of the default mode of callbacks as occurring asynchronously - but the conservative thing is still to assume synchronous here.
  • Might be worth thinking about how reads occur - for comparison versus assignability.
  • So what are the risks and open questions?
    • Obviously there are breaks - we don't know how big they are.
    • Should we look at objects and classes with objects?
      • Thinking o
    • Should we make exceptions to Promise.prototype.then?
      • What about await on those?

        async function f() {
            let foo1: "a" | "b" = "b";
            await Promise.resolve().then(() => {
                foo1 = "a";
            });
        
            if (foo1 === "a") {
                // TypeScript assumes that this is impossible
                // because it doesn't understand that `foo1`
                // might have been assigned to in the callback.
                console.log("foo1 is a");
            }
        }
  • Implementation: creating a synthetic join point for all the function arguments to a call node.
  • Do we need to consider that each function's control flow needs to be dominated by prior arguments?
    • Well, no, the arguments could be invoked in an arbitrary order.

    • Really the more conservative thing is to assume not just that the function expressions may have been called after the call, but also that

    • Example

      function foo() {
          /*Call_pre*/
          someCall(/*A_pre*/ () => {... /*A_post*/ }, /*B_pre*/ () => {... /*B_post*/});
          /*Call_post*/
      }
      • We need to assume that A_post is a possible antecedent of A_pre and B_pre, and that B_post is a possible antecedent of A_pre and B_pre.

        digraph {
            rankdir="BT";
        
            A_pre -> Call_pre
            A_pre -> A_pre
            A_pre -> B_pre
            
            B_pre -> Call_pre
            B_pre -> B_pre
            B_pre -> A_pre
        
            A_post -> A_pre
            B_post -> B_pre
        
            Call_post -> A_post
            Call_post -> B_post
            Call_post -> Call_pre
        }
          graph BT;
              A_pre --> Call_pre
              A_pre --> A_pre
              A_pre --> B_pre
              
              B_pre --> Call_pre
              B_pre --> B_pre
              B_pre --> A_pre
        
              A_post --> A_pre
              B_post --> B_pre
        
              Call_post --> A_post
              Call_post --> B_post
              Call_post --> Call_pre
          
        
        Loading
  • Ultimately we gotta see what this affects - but also, how does this work on our own codebase?
    • Trying it out, we don't seem to have any code that has this pattern. We'll need to try it out on other real-world code.
@DanielRosenwasser DanielRosenwasser added the Design Notes Notes from our design meetings label Jun 3, 2024
@DanielRosenwasser
Copy link
Member Author

See #58729 for details.

@jcalz
Copy link
Contributor

jcalz commented Jun 4, 2024

mystery(() => {}); would be quite mysterious if it can reassign x without an assignment in the body of the callback. I assume this is a typo?

@ethanresnick
Copy link
Contributor

@DanielRosenwasser I just wanna say thank you and the team for publishing these design notes. I always learn something from them.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Design Notes Notes from our design meetings
Projects
None yet
Development

No branches or pull requests

3 participants