-
Notifications
You must be signed in to change notification settings - Fork 155
Add Byref Initializers of Byref Variables #89
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
Conversation
jayrm
commented
Jun 20, 2018
- added hResolveRefToRefInitializer() called from hCheckAndBuildByrefInitializer()
- allows initializing a reference variable by referring to another reference variable.
- was discussed briefly in forum topic Dim Byref Syntax
- hCMPI was generate invalid ASM if operands were IMM (constant) and OFS (offset to shared/static) - dim shared X as integer: if( @x <> 0 ) then ... to produce the error
src/compiler/emit_x86.bas
Outdated
| ostr = "cmp " + dst + COMMA + src | ||
| outp ostr | ||
|
|
||
| '' immediate and offset? it's invalid for CMP so use a temp reg |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Somehow this issue seemed familiar - cc8638c
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's perfect. Almost the same problem. Will revert emit_x86.bas and let IR handle it.
src/compiler/emit_x86.bas
Outdated
| '' immediate and offset? it's invalid for CMP so use a temp reg | ||
| if( (svreg->typ = IR_VREGTYPE_IMM) and (dvreg->typ = IR_VREGTYPE_OFS) ) then | ||
|
|
||
| dim reg as integer, isfree as integer, rname as string |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
inner rname is shadowing the outer rname here, might be better to split into an extra function
src/compiler/parser-decl-var.bas
Outdated
|
|
||
| if( expr andalso astIsDEREF( expr ) ) then | ||
| if( expr->l andalso astIsVAR( expr->l ) ) then | ||
| if( expr->l->sym andalso symbIsRef( expr->l->sym ) ) then |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It could be helpful to also check symbIsVar, since the symbIsRef flag can apply to other symbol types too, i.e. procedures for "return byref", and looking at astNewVAR() it refers to the procedure symbol in case of function pointers.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added symbIsVar check. Thanks. I think for the correct sym.
| @@ -1,4 +1,4 @@ | |||
| ' TEST_MODE : COMPILE_ONLY_FAIL | |||
| ' TEST_MODE : COMPILE_ONLY_OK | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we can probably remove this now, since the scenario is better covered by the new unit test
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
- Fix bad code generated for comparisons such as IF @globalvar = 0 THEN - add tests to /tests/expressions/address-comparison.bas