parser: Don't disallow mixing RETURN/FUNCTION= in byref functions

Author: dkl

changelog.txt

@@ -63,6 +63,7 @@ Version 1.01.0
 - 0.90.0 regression: Compiler crash during error recovery for expressions like 'type<UDT>().field' where initializer values are missing
 - When passing a constant zero of some integer to an overloaded procedure, overloads with integer parameters will now be preferred over overloads with pointers (literal 0 can still be passed to pointer parameters in this case though, by casting it to the pointer type)
 - Literal zeroes with [U]Byte or [U]Short type (or the 16bit Integer in -lang qb) can now be passed to pointer parameters of overloaded procedures (now it's possible to call Bsave() with 0 source buffer in -lang qb again -- it also was affected by this bug since FB 0.90)
+- RETURN and FUNCTION= couldn't be used together in byref functions returning an UDT with constructor. This check is now only applied to functions returning byval.
 
 
 Version 1.00.0 (former 0.91.0):

src/compiler/parser-compound.bas

@@ -156,20 +156,20 @@ sub cEndStatement( )
 end sub
 
 private function hCheckForCtorResult( ) as integer
-	function = FB_ERRMSG_OK
-	if( symbHasCtor( parser.currproc ) ) then
-		if( symbGetProcStatReturnUsed( parser.currproc ) ) then
+	if( symbGetProcStatReturnUsed( parser.currproc ) ) then
+		if( symbHasCtor( parser.currproc ) and _
+		    (not symbProcReturnsByref( parser.currproc )) ) then
 			'' EXIT FUNCTION cannot be allowed in combination
-			'' with RETURN and a ctor result, because it would
-			'' not call the result constructor.
-			function = FB_ERRMSG_MISSINGRETURNFORCTORRESULT
-		else
-			'' EXIT FUNCTION used, and no RETURN yet:
-			'' make it behave like FUNCTION=, to ensure the result ctor
-			'' is called at the top.
-			symbSetProcStatAssignUsed( parser.currproc )
+			'' with RETURN and a ctor result for byval functions,
+			'' because it would not call the result constructor.
+			return FB_ERRMSG_MISSINGRETURNFORCTORRESULT
 		end if
 	end if
+
+	'' EXIT FUNCTION used; mark the function as if FUNCTION= was used,
+	'' to ensure the result will be constructed at the top if needed.
+	symbSetProcStatAssignUsed( parser.currproc )
+	function = FB_ERRMSG_OK
 end function
 
 '':::::

src/compiler/parser-proccall.bas

@@ -46,19 +46,20 @@ function cAssignFunctResult( byval is_return as integer ) as integer
 
 	has_ctor = symbHasCtor( parser.currproc )
 	has_defctor = symbHasDefCtor( parser.currproc )
+	var returns_byref = symbProcReturnsByref( parser.currproc )
 
 	'' RETURN?
 	if( is_return ) then
 		if( symbGetProcStatAssignUsed( parser.currproc ) ) then
-			if( has_defctor ) then
+			if( has_defctor and (not returns_byref) ) then
 				errReport( FB_ERRMSG_RETURNMIXEDWITHASSIGN )
 			end if
 		end if
 
 		symbSetProcStatReturnUsed( parser.currproc )
 	else
 		if( symbGetProcStatReturnUsed( parser.currproc ) ) then
-			if( has_defctor ) then
+			if( has_defctor and (not returns_byref) ) then
 				errReport( FB_ERRMSG_ASSIGNMIXEDWITHRETURN )
 			end if
 		end if
@@ -80,7 +81,7 @@ function cAssignFunctResult( byval is_return as integer ) as integer
 	'' is allowed, no implicit ADDROF is done, just like with BYREF params.
 
 	'' Returning BYREF and no explicit BYVAL given?
-	if( symbProcReturnsByref( parser.currproc ) and (not hMatch( FB_TK_BYVAL )) ) then
+	if( returns_byref and (not hMatch( FB_TK_BYVAL )) ) then
 		'' BYREF return, must be able to do addrof on the expression
 		'' (this disallows expressions like constants, BOPs, @ UOP, ...)
 		rhs = cVarOrDeref( FB_VAREXPROPT_ISEXPR )

tests/functions/return-byref.bas

@@ -806,6 +806,75 @@ namespace cxxMangling
 	end sub
 end namespace
 
+namespace returnVsFunction
+	'' For functions returning BYREF, both RETURN and FUNCTION= should be
+	'' allowed to assign results, no matter what result type it is, since
+	'' it's just a pointer, and the function doesn't actually have to call
+	'' any constructor.
+
+	type CtorUdt
+		i as integer
+		declare constructor( )
+	end type
+
+	constructor CtorUdt( )
+	end constructor
+
+	dim shared as integer ireturn, ifunction
+	dim shared as CtorUdt ctorudtreturn, ctorudtfunction
+
+	function getInteger1( ) byref as integer
+		return ireturn
+		function = ifunction
+	end function
+
+	function getInteger2( ) byref as integer
+		function = ifunction
+		return ireturn
+	end function
+
+	function getInteger3( ) byref as integer
+		exit function
+		return ireturn
+	end function
+
+	function getInteger4( ) byref as integer
+		return ireturn
+		exit function
+	end function
+
+	function getCtorUdt1( ) byref as CtorUdt
+		return ctorudtreturn
+		function = ctorudtfunction
+	end function
+
+	function getCtorUdt2( ) byref as CtorUdt
+		function = ctorudtfunction
+		return ctorudtreturn
+	end function
+
+	function getCtorUdt3( ) byref as CtorUdt
+		exit function
+		return ctorudtreturn
+	end function
+
+	function getCtorUdt4( ) byref as CtorUdt
+		return ctorudtreturn
+		exit function
+	end function
+
+	sub test cdecl( )
+		CU_ASSERT( @(getInteger1( )) = @ireturn )
+		CU_ASSERT( @(getInteger2( )) = @ireturn )
+		CU_ASSERT( @(getInteger3( )) = 0 )
+		CU_ASSERT( @(getInteger4( )) = @ireturn )
+		CU_ASSERT( @(getCtorUdt1( )) = @ctorudtreturn )
+		CU_ASSERT( @(getCtorUdt2( )) = @ctorudtreturn )
+		CU_ASSERT( @(getCtorUdt3( )) = 0 )
+		CU_ASSERT( @(getCtorUdt4( )) = @ctorudtreturn )
+	end sub
+end namespace
+
 private sub ctor( ) constructor
 	fbcu.add_suite( "tests/functions/return-byref" )
 	fbcu.add_test( "returning globals", @returnGlobal.test )
@@ -830,6 +899,7 @@ private sub ctor( ) constructor
 	fbcu.add_test( "explicit BYVAL", @explicitByval.test )
 	fbcu.add_test( "returning a forward ref", @protoReturningFwdref.test )
 	fbcu.add_test( "C++ mangling", @cxxMangling.test )
+	fbcu.add_test( "ReturnVsFunction", @returnVsFunction.test )
 end sub
 
 end namespace