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

Disallow referencing the if variable in the else branch #20835

Open
Bolpat opened this issue Feb 7, 2025 · 5 comments
Open

Disallow referencing the if variable in the else branch #20835

Bolpat opened this issue Feb 7, 2025 · 5 comments

Comments

@Bolpat
Copy link
Contributor

Bolpat commented Feb 7, 2025

A variable declared in the condition of an if statement is considered not to be in scope in the else branch. That makes sense insofar as the variable (usually) has a trivial/zero value. However, that might lead to accidentally referencing something else, such as a member variable or a global variable, by programmers coming from other languages where an if variable declaration either extends into the else branch, too, (e.g. C++) or even the whole end of the scope containing the if statement (e.g. C#). That can lead to subtle bugs. It would be preferable to require accessing these via the usual disambiguation mechanism, i.e. require using this.Identifier or .Identifier even if the local variable is technically not in scope. Code that declares a new variable of the same name as the if variable should be unaffected: It’s clear that the programmer understood that the if variable is not in scope and it’s clear to the reader which variable it refers to, as a declaration in the else branch is the closest one.

// Current behavior
int y;
void main()
{
    if (int y = 0) { }
    else { y = 10; } // sets global y
    assert(.y == 10); // passes
}
// Behavior after fix
int y;
void main()
{
    if (int y = 0) { }
    else { y = 10; } // Error: Use `.y` to access global variable `y`. Note: `int y` from `if` is not in scope.

    if (int y = 0) { }
    else { int y = 10; } // still good
}
@muscar
Copy link
Contributor

muscar commented Feb 9, 2025

@Bolpat I can have a go at this.

@ghost
Copy link

ghost commented Feb 10, 2025

I think this is more the job a linter. The reason for that is that I'm thinking about how this could be implemented and I see a depleasant problem that is that IdentExps sema would be slightly slower in the else scope.

Also what if one day D adds "inline-variables" as a new kind of primary expression. That would mean that every variables used in the if-condition would have to be checked.

@Bolpat
Copy link
Contributor Author

Bolpat commented Feb 10, 2025

Also what if one day D adds "inline-variables" as a new kind of primary expression. That would mean that every variables used in the if-condition would have to be checked.

I’m not sure what that means. You might mean something like C# if (Expr() is string str) which declares a variable str? It depends how the scope of these would be defined. In C#, their scope isn’t limited to the if statement (including else) but extends to the end of the scope in which the if resides. D would likely never do that. I’m aware of a proposal that would allow if (int x = Expr(); x > 0), but there it clearly says the scope of x includes the else branch.

You might also mean something like int x = (int y = expr(), y + y/2); which declares a variable in a comma expression. I’d imagine the scope of such a variable (y in this example) would extend to the closing parenthesis only.

If I understood you correctly, it’s not an issue.

@muscar
Copy link
Contributor

muscar commented Feb 15, 2025

@Bolpat is this still an open task then?

@Bolpat
Copy link
Contributor Author

Bolpat commented Feb 20, 2025

is this still an open task then?

I don’t see why it wouldn’t.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants