Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP
Browse files

Fix missing implicit DEREF for BYREF-proc calls as lvalues

  • Loading branch information...
commit 6fe220394dbf746fafb019e206f652d524a341e5 1 parent d28bbc1
@dkl dkl authored
View
3  src/compiler/ast-node-arg.bas
@@ -660,6 +660,7 @@ private sub hUDTPassByval _
n->l = astBuildCallResultVar( arg )
hByteByByte( param, n )
else
+ assert( symbProcReturnsByref( arg->sym ) = FALSE )
'' CALL with result in registers, patch the type
astSetType( arg, symbGetProcRealType( arg->sym ), _
symbGetProcRealSubtype( arg->sym ) )
@@ -787,6 +788,8 @@ private function hCheckUDTParam _
case FB_PARAMMODE_BYREF
if( astIsCALL( arg ) ) then
if( symbProcReturnsOnStack( arg->sym ) = FALSE ) then
+ assert( symbProcReturnsByref( arg->sym ) = FALSE )
+
'' Returning in registers, passed to a BYREF param
'' Create a temp var and pass that
tmp = symbAddTempVar( astGetDatatype( arg ), arg->subtype )
View
2  src/compiler/ast-node-assign.bas
@@ -531,6 +531,8 @@ function astNewASSIGN _
'' Returning on stack, copy from the temp result var
r = astBuildCallResultVar( r )
else
+ assert( symbProcReturnsByref( r->sym ) = FALSE )
+
'' Returning in registers, patch the types and do a normal ASSIGN
ldfull = symbGetProcRealType( r->sym )
ldtype = typeGet( ldfull )
View
26 src/compiler/ast-node-call.bas
@@ -218,6 +218,12 @@ function astLoadCALL( byval n as ASTNODE ptr ) as IRVREG ptr
if( astGetDataType( n ) = FB_DATATYPE_VOID ) then
vr = NULL
else
+ '' When returning BYREF the CALL's dtype should have
+ '' been remapped by astBuildByrefResultDeref()
+ assert( iif( symbProcReturnsByref( proc ), _
+ astGetDataType( n ) = typeGetDtAndPtrOnly( symbGetProcRealType( proc ) ), _
+ TRUE ) )
+
vr = irAllocVREG( typeGetDtAndPtrOnly( symbGetProcRealType( proc ) ), _
symbGetProcRealSubtype( proc ) )
@@ -368,6 +374,7 @@ function astBuildCallResultUdt( byval expr as ASTNODE ptr ) as ASTNODE ptr
assert( astIsCALL( expr ) )
assert( astGetDataType( expr ) = FB_DATATYPE_STRUCT )
+ assert( symbProcReturnsByref( expr->sym ) = FALSE )
if( symbProcReturnsOnStack( expr->sym ) ) then
'' UDT returned in temp var already, just access that one
@@ -384,12 +391,29 @@ function astBuildCallResultUdt( byval expr as ASTNODE ptr ) as ASTNODE ptr
end function
function astBuildByrefResultDeref( byval expr as ASTNODE ptr ) as ASTNODE ptr
- assert( astIsCALL( expr ) and symbIsProc( expr->sym ) )
+ assert( astIsCALL( expr ) )
+ assert( symbProcReturnsByref( expr->sym ) )
'' Do an implicit DEREF with the function's type, and remap the CALL
'' node's type to the pointer, so the AST is consistent even if that
'' DEREF gets optimized out.
astSetType( expr, symbGetProcRealType( expr->sym ), _
symbGetProcRealSubtype( expr->sym ) )
+
function = astNewDEREF( expr )
end function
+
+function astIsByrefResultDeref( byval expr as ASTNODE ptr ) as integer
+ function = FALSE
+ if( astIsDEREF( expr ) ) then
+ if( astIsCALL( expr->l ) ) then
+ function = symbProcReturnsByref( expr->l->sym )
+ end if
+ end if
+end function
+
+function astRemoveByrefResultDeref( byval expr as ASTNODE ptr ) as ASTNODE ptr
+ assert( astIsByrefResultDeref( expr ) )
+ function = expr->l
+ astDelNode( expr )
+end function
View
2  src/compiler/ast.bi
@@ -1234,6 +1234,8 @@ declare function astGetOFFSETChildOfs _
declare function astBuildCallResultVar( byval expr as ASTNODE ptr ) as ASTNODE ptr
declare function astBuildCallResultUdt( byval expr as ASTNODE ptr ) as ASTNODE ptr
declare function astBuildByrefResultDeref( byval expr as ASTNODE ptr ) as ASTNODE ptr
+declare function astIsByrefResultDeref( byval expr as ASTNODE ptr ) as integer
+declare function astRemoveByrefResultDeref( byval expr as ASTNODE ptr ) as ASTNODE ptr
declare sub astGosubAddInit( byval proc as FBSYMBOL ptr )
View
39 src/compiler/parser-proccall.bas
@@ -322,23 +322,46 @@ function cProcCall _
fbSetPrntOptional( FALSE )
- '' StrIdxOrMemberDeref?
if( is_propset = FALSE ) then
+ '' Function returns BYREF?
+ if( symbProcReturnsByref( sym ) ) then
+ procexpr = astBuildByrefResultDeref( procexpr )
+ end if
+
+ '' StrIdxOrMemberDeref?
procexpr = cStrIdxOrMemberDeref( procexpr )
- end if
- '' if it's a SUB, the expr will be NULL
- if( procexpr = NULL ) then
- exit function
+ '' if it's a SUB, the expr will be NULL
+ if( procexpr = NULL ) then
+ exit function
+ end if
end if
- dtype = astGetDataType( procexpr )
-
'' not a function? (because StrIdxOrMemberDeref())
if( astIsCALL( procexpr ) = FALSE ) then
- return procexpr
+ '' And not a DEREF( CALL( function-with-byref-result ) ) either?
+ if( astIsByrefResultDeref( procexpr ) = FALSE ) then
+ '' Cannot ignore this
+ return procexpr
+ end if
+
+ select case( lexGetToken( ) )
+ case FB_TK_STMTSEP, FB_TK_EOL, FB_TK_EOF, _
+ FB_TK_COMMENT, FB_TK_REM
+ '' It seems like the result is being ignored,
+ '' i.e. no assignment following
+
+ case else
+ return procexpr
+ end select
+
+ '' Remove the DEREF and turn it into a plain CALL,
+ '' whose result can be ignored.
+ procexpr = astRemoveByrefResultDeref( procexpr )
end if
+ dtype = astGetDataType( procexpr )
+
'' can proc's result be skipped?
if( dtype <> FB_DATATYPE_VOID ) then
if( typeGetClass( dtype ) <> FB_DATACLASS_INTEGER ) then
View
99 tests/functions/return-byref.bas
@@ -394,7 +394,11 @@ namespace ignoreResult
sub test cdecl( )
CU_ASSERT( calls = 0 )
f( )
- CU_ASSERT( calls = 1 )
+ f( ) 'comment
+ f( ) rem rem
+ f( ) : 'stmtsep
+ f( ) : f( ) 'stmtsep
+ CU_ASSERT( calls = 6 )
end sub
end namespace
@@ -562,6 +566,99 @@ namespace explicitByval
end sub
end namespace
+namespace byrefResultAsLvalue
+ type UDT
+ as integer a, b
+ end type
+
+ dim shared dat(0 to 2) as zstring * 32 = { "a", "b", "c" }
+ dim shared i as integer
+
+ function f1( ) byref as zstring
+ function = dat( i )
+ end function
+
+ function f2( byval j as integer ) byref as zstring
+ function = dat( j )
+ end function
+
+ function f3( ) byref as UDT
+ static as UDT x1 = ( 12, 34 ), x2 = ( 56, 78 )
+ if( i ) then
+ return x1
+ end if
+ function = x2
+ end function
+
+ function f4( byval j as integer ) byref as UDT
+ static as UDT x1 = ( 12, 34 ), x2 = ( 56, 78 )
+ if( j ) then
+ return x1
+ end if
+ function = x2
+ end function
+
+ sub test cdecl( )
+ i = 0 : CU_ASSERT( f1( ) = "a" )
+ i = 1 : CU_ASSERT( f1( ) = "b" )
+ i = 2 : CU_ASSERT( f1( ) = "c" )
+ i = 1 : f1( ) = "y" '' Note: must be careful; it's like *pzstr = "foo", the length is unchecked
+ i = 0 : CU_ASSERT( f1( ) = "a" )
+ i = 1 : CU_ASSERT( f1( ) = "y" )
+ i = 2 : CU_ASSERT( f1( ) = "c" )
+ i = 0 : f1( ) = "x"
+ i = 2 : f1( ) = "z"
+ i = 0 : CU_ASSERT( f1( ) = "x" )
+ i = 1 : CU_ASSERT( f1( ) = "y" )
+ i = 2 : CU_ASSERT( f1( ) = "z" )
+
+ CU_ASSERT( f2( 0 ) = "a" )
+ CU_ASSERT( f2( 1 ) = "b" )
+ CU_ASSERT( f2( 2 ) = "c" )
+ (f2( 1 )) = "y"
+ CU_ASSERT( f2( 0 ) = "a" )
+ CU_ASSERT( f2( 1 ) = "y" )
+ CU_ASSERT( f2( 2 ) = "c" )
+ (f2( 0 )) = "x"
+ (f2( 2 )) = "z"
+ CU_ASSERT( f2( 0 ) = "x" )
+ CU_ASSERT( f2( 1 ) = "y" )
+ CU_ASSERT( f2( 2 ) = "z" )
+
+ i = 1 : CU_ASSERT( f3( ).a = 12 )
+ i = 1 : CU_ASSERT( f3( ).b = 34 )
+ i = 0 : CU_ASSERT( f3( ).a = 56 )
+ i = 0 : CU_ASSERT( f3( ).b = 78 )
+ i = 1 : f3( ).a = 0
+ i = 0 : f3( ).b = 0
+ i = 1 : CU_ASSERT( f3( ).a = 0 )
+ i = 1 : CU_ASSERT( f3( ).b = 34 )
+ i = 0 : CU_ASSERT( f3( ).a = 56 )
+ i = 0 : CU_ASSERT( f3( ).b = 0 )
+ i = 0 : f3( ) = type<UDT>( 22, 33 )
+ i = 1 : CU_ASSERT( f3( ).a = 0 )
+ i = 1 : CU_ASSERT( f3( ).b = 34 )
+ i = 0 : CU_ASSERT( f3( ).a = 22 )
+ i = 0 : CU_ASSERT( f3( ).b = 33 )
+
+ CU_ASSERT( f4( 1 ).a = 12 )
+ CU_ASSERT( f4( 1 ).b = 34 )
+ CU_ASSERT( f4( 0 ).a = 56 )
+ CU_ASSERT( f4( 0 ).b = 78 )
+ (f4( 1 ).a) = 0
+ (f4( 0 )).b = 0
+ CU_ASSERT( f4( 1 ).a = 0 )
+ CU_ASSERT( f4( 1 ).b = 34 )
+ CU_ASSERT( f4( 0 ).a = 56 )
+ CU_ASSERT( f4( 0 ).b = 0 )
+ (f4( 0 )) = type<UDT>( 22, 33 )
+ CU_ASSERT( f4( 1 ).a = 0 )
+ CU_ASSERT( f4( 1 ).b = 34 )
+ CU_ASSERT( f4( 0 ).a = 22 )
+ CU_ASSERT( f4( 0 ).b = 33 )
+ end sub
+end namespace
+
private sub ctor( ) constructor
fbcu.add_suite( "tests/functions/return-byref" )
fbcu.add_test( "returning globals", @returnGlobal.test )
View
3  todo.txt
@@ -93,6 +93,9 @@ o -exx should catch...
o BYREF function results
- hPatchByvalResultToSelf() should probably check symbProcReturnsByref() now
+ - "f( 1 ) = 2" will be parsed as "f( (1) = 2 )", could parse BYREF proc params
+ differently, e.g. with cEqInParensOnlyExpr(), or just always require ()'s
+ to call BYREF procs?
[ ] quirks:
[ ] OPEN ... FOR is not checking for mode
Please sign in to comment.
Something went wrong with that request. Please try again.