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

Pass-by-ref vars don't account for early-outing; should merge types instead of overwriting #4602

Merged
merged 1 commit into from
Nov 2, 2021

Conversation

arichard4
Copy link
Contributor

Here's the current output for 0970_ref_param_types.php.expected:

ref.php:13 PhanDebugAnnotation @phan-debug-var requested for variable $var - it has union type 1|2|3
ref.php:29 PhanDebugAnnotation @phan-debug-var requested for variable $var2 - it has union type 6
ref.php:44 PhanDebugAnnotation @phan-debug-var requested for variable $var3 - it has union type 9
ref.php:60 PhanDebugAnnotation @phan-debug-var requested for variable $var4 - it has union type 12
ref.php:71 PhanDebugAnnotation @phan-debug-var requested for variable $var5 - it has union type 14
ref.php:93 PhanDebugAnnotation @phan-debug-var requested for variable $var6 - it has union type 16

As-is, PassByReferenceVariable doesn't account for early-outing; it assumes that pass-by-ref variables take on the types the last time it was written to (or the merged value for multiple branches which don't early-out).

I'd explored instead deferring updating $this->element's type until the function's end, a return, or a throw, and wiping any previous values if all possible exit paths modified the variable; the problem with that is illustrated with $var6 in the test cases- an exception can interrupt execution even in a function that doesn't directly throw or catch it.

There are two cases where this will introduce bugs; I am assuming it's better for the UnionType to have false positives than false negatives.

$var6 = outer();
'@phan-debug-var $var6';

function simple(&$var) {
	$var = 18;
}

$var7 = 17;
simple($var7);
'@phan-debug-var $var7'; // 17|18; should be 18

function dupe(&$var) {
	$var = 19;
	$var = 20;
}

dupe($var8);
'@phan-debug-var $var8'; // 19|20; should be 20

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

Successfully merging this pull request may close these issues.

None yet

2 participants