Skip to content

CFA should consider branch flags #20497

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

Open
tinganho opened this issue Dec 6, 2017 · 4 comments
Open

CFA should consider branch flags #20497

tinganho opened this issue Dec 6, 2017 · 4 comments
Labels
Needs Proposal This issue needs a plan that clarifies the finer details of how it could be implemented. Suggestion An idea for TypeScript

Comments

@tinganho
Copy link
Contributor

tinganho commented Dec 6, 2017

This is a suggestion to improve the CFA. It would be good if TypeScript considered branch flags when analysing code paths.

function f() {
    let text: string;
    let hasTakenVerySpecificBranch = false;
    if (0.5 < Math.random()) {
        if (0.5 < Math.random()) {
            if (0.5 < Math.random()) {
                hasTakenVerySpecificBranch = true;
                text = '';
            }
        }
    }
    if (!hasTakenVerySpecificBranch) {
        text = ''
    }
    console.log(text); // Error 'text' is used before assigned.
}

We assign text on all possible code paths above. And it shouldn't error above.

A branch flag is simply a boolean variable that is considered when doing the control flow analysis.

@RyanCavanaugh RyanCavanaugh added Needs Proposal This issue needs a plan that clarifies the finer details of how it could be implemented. Suggestion An idea for TypeScript labels Dec 6, 2017
@RyanCavanaugh
Copy link
Member

Unless there's some simple way to do this that I'm missing, it seems almost incredibly complex to track all the possible interactions of variables as the function runs.

@tinganho
Copy link
Contributor Author

tinganho commented Dec 6, 2017

Maybe you are thinking about a more generic case of taking into consideration all variable assignment types?

I was thinking about just :

  1. Mark variables that is initialized to false as candidates for branch flags.
  2. Detect true assignment branches.
  3. Check if there is catch-all-but-true branch. (if (!hasTakenVerySpecificBranch))
  4. If and only if a variable is assigned on catch-all-but-true and all true branches then narrow the types to the corresponding assigned types.

I detected 3 places in my code that could benefit from this. The workaround I do right now is to define the variable as string | undefined and use the non-null assertion operator everywhere. This can get ugly quickly if you use the variable in a lot of places.

I think the pattern of using branch flags is quite common. The variable hasTakenVerySpecificBranch is really part of the control flow like the if, else if and else statements are and should be treated equally in CFA.

@tinganho
Copy link
Contributor Author

tinganho commented Dec 6, 2017

Hmm, after thinking about it a bit, a branch flag can have have more values than just two:

const enum Animal {
    Unknown,
    Cow,
    Pig,
    Chicken,
}

interface IAnimal {
    type: Animal;
    age: number;
    sound: string;
    legs: 2 | 4;
}

function detectAnimal(sound: string, age: number): IAnimal {
    let type = Animal.Unknown;
    let legs: 2 | 4;
    if (age > 1) {
        if (sound === 'quack quack') {
            type = Animal.Chicken;
            legs = 2;
        }
        if (sound === 'nöff nöff') {
            type = Animal.Pig;
            legs = 4;
        }
    }
    else {
        throw new Error('Animals do not make sound at age lower than 1.');
    }
    if (type !== Animal.Chicken && type !== Animal.Pig) {
        legs = 4;
    }
    return {
        type,
        age,
        sound,
        legs, // Error 'legs' is used before assigned.
    }
}

I could probably put up a more detailed proposal.

@ejose19
Copy link

ejose19 commented Apr 28, 2020

This could be useful for the following cases:

const condition = true; // <-- can be obtained differently, the point is that it's a constant
let message: string;

if (condition) message = 'hello';

// Some shared code that runs independently of condition

if (condition) console.log(message); // <--- this gives error "message is not defined"

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Needs Proposal This issue needs a plan that clarifies the finer details of how it could be implemented. Suggestion An idea for TypeScript
Projects
None yet
Development

No branches or pull requests

3 participants