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

Fix op_if/unless/with/without coercion #383

Closed
wants to merge 4 commits into from
Closed

Fix op_if/unless/with/without coercion #383

wants to merge 4 commits into from

Conversation

tisonkun
Copy link
Member

@tisonkun tisonkun commented Dec 7, 2017

Sample code:

my $foo := 42;
my int $bar := $foo || 200;
say($bar);

Before it says '0' and now '42'.
The problem is we generate $foo || 200 as

- QAST::Op(unless &infix:<||>)  ||
  - QAST::Var(lexical $bar)
  - QAST::IVal(200)

and thus $bar only be a condition. If we want to return its value,
we forget to coerce its kind.

Now this fix only apply on op_unless. If applies on the four ops
handled this block, it fails some tests in t/qast/01-qast.t.
For details

t/qast/01-qast.t                     (Wstat: 0 Tests: 166 Failed: 3)
  Failed tests:  156, 162, 165

Sample code:
```
my $foo := 42;
my int $bar := $foo || 200;
say($bar);
```

Before it says '0' and now '42'.
The problem is we generate $foo || 200 as

- QAST::Op(unless &infix:<||>)  ||
  - QAST::Var(lexical $bar)
  - QAST::IVal(200)

and thus $bar only be a condition. If we want to return its value,
we forget to coerce its kind.

Now this fix only apply on op_unless. If applies on the four ops
handled this block, it fails some tests in t/qast/01-qast.t.
@tisonkun tisonkun changed the title Fix op_unless coercion [WIP]Fix op_unless coercion Dec 7, 2017
@tisonkun
Copy link
Member Author

tisonkun commented Dec 7, 2017

No, this break

my $foo := 0;
my int $bar := $foo || 400;
say($bar);

to say 0. I will work on it.

@tisonkun tisonkun changed the title [WIP]Fix op_unless coercion Fix op_if/unless/with/withour coercion Dec 7, 2017
@tisonkun tisonkun changed the title Fix op_if/unless/with/withour coercion Fix op_if/unless/with/without coercion Dec 7, 2017
@tisonkun
Copy link
Member Author

tisonkun commented Dec 7, 2017

Now we generate correct condition IR, it's quite like a copy from JVM backend.

a || b

ifFalse a goto L1
then-clause
goto L2
L1:
{ coerce if oprands == 2 and needed }
else-clause
L2:
...

(nqp::defined($*WANT) ?? $*WANT !! $MVM_reg_obj));
$fix_coercion := $coercion;
$regalloc.release_register($res_reg, $res_kind);
$res_reg := $regalloc.fresh_register($coercion.result_kind);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You need to set the register in $res_reg at runtime instead of overwriting $res_reg at compile time. We might succeed and not do the coercion at all

);
}

my $fix_coercion;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No need for a variable, just add the coercion when we are checking for the 3 operand form.

@zoffixznet zoffixznet removed their request for review December 7, 2017 11:46
@tisonkun tisonkun closed this Dec 7, 2017
@tisonkun tisonkun deleted the patch-1 branch December 7, 2017 13:39
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