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

if-related invariants not erased before handling else branch #363

Closed
quasilyte opened this issue Mar 20, 2020 · 3 comments · Fixed by #1058
Closed

if-related invariants not erased before handling else branch #363

quasilyte opened this issue Mar 20, 2020 · 3 comments · Fixed by #1058
Assignees
Labels
bug Something isn't working

Comments

@quasilyte
Copy link
Contributor

quasilyte commented Mar 20, 2020

Code Example

<?php

class C {
  /***/
  public function example() {}
}

function f($c) {
  if ($c instanceof C) {
  } else {
    $c->example();
  }
}

Actual Behavior

No warnings.

Expected Behavior

Calling to undefined method, since $c is not C-typed.

@quasilyte quasilyte added the bug Something isn't working label Mar 20, 2020
@quasilyte
Copy link
Contributor Author

quasilyte commented Mar 20, 2020

This is not only about instanceof but rather about all things recorder from if.

function f() {
  if (isset($x)) {
  } else {
    var_dump($x);
  }
}

This code gives no warning about undefined $x.

Looks like we handle elseif correctly, but if it's else if then we have the same issue:

function f() {
  // No warnings
  if (isset($x)) {
  } else if (isset($y)) {
    var_dump($x);
  } else {
    var_dump($y);
  }
}

@quasilyte
Copy link
Contributor Author

Another observation:

if (isset($x) && $foo = 10) {}

var_dump($foo);

The code above does not generate any warnings because we treat everything in if condition as evaluated unconditionally.

It should probably report usage $foo as "might not be defined".

quasilyte added a commit that referenced this issue Mar 20, 2020
This change attempts to improve handleIf behavior.

We introduce implicit kind of vars to express automatically
defined vars that can be handled in a special way.

andWalker.varsToDelete are removed because we're running
all branches in a separate block contexts. No extra
cleanup is required.

Changed code requires to evaluate if statement condition
separately, so we can get variables defined inside it
into the parent block context.
assignWalker type does that.

Fixes #369
Fixes #363
Fixes #370

Signed-off-by: Iskander Sharipov <quasilyte@gmail.com>
quasilyte added a commit that referenced this issue Mar 20, 2020
This change attempts to improve handleIf behavior.

We introduce implicit kind of vars to express automatically
defined vars that can be handled in a special way.

andWalker.varsToDelete are removed because we're running
all branches in a separate block contexts. No extra
cleanup is required.

Changed code requires to evaluate if statement condition
separately, so we can get variables defined inside it
into the parent block context.
assignWalker type does that.

Fixes #369
Fixes #363
Fixes #370

Signed-off-by: Iskander Sharipov <quasilyte@gmail.com>
@i582
Copy link
Contributor

i582 commented Aug 25, 2021

Done by #1058

@i582 i582 closed this as completed Aug 25, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
2 participants