Fix #767: Show proper error for illegal byref result assignments

(and not just a warning)

(cherry picked from commit ae86415)

Author: dkl

changelog.txt

@@ -14,6 +14,7 @@ Version 1.02.1
 - bindings: Various previously untranslated (or wrongly translated) #defines in SDL2, X11, crt/pthread, Windows API
 - Windows API binding: DirectX headers missed some declarations and some didn't compile
 - crt/string.bi, crt/mem.bi: Added CONSTs to function declarations
+- #767: Illegal byref result assignments will now cause a proper error message, not just a warning
 
 
 Version 1.02.0

src/compiler/ast-node-assign.bas

@@ -370,6 +370,18 @@ function astCheckASSIGNToType _
 	astDelTree( l )
 end function
 
+'' Check whether it's ok to assign
+''   l = r
+'' where l is a reference.
+function astCheckByrefAssign _
+	( _
+		byval ldtype as integer, _
+		byval lsubtype as FBSYMBOL ptr, _
+		byval r as ASTNODE ptr _
+	) as integer
+	function = (typeCalcMatch( ldtype, lsubtype, FB_PARAMMODE_BYREF, r->dtype, r->subtype ) > 0)
+end function
+
 private function hShallowCopy _
 	( _
 		byval l as ASTNODE ptr, _

src/compiler/ast.bi

@@ -904,6 +904,13 @@ declare function astCheckASSIGNToType _
 		byval r as ASTNODE ptr _
 	) as integer
 
+declare function astCheckByrefAssign _
+	( _
+		byval ldtype as integer, _
+		byval lsubtype as FBSYMBOL ptr, _
+		byval r as ASTNODE ptr _
+	) as integer
+
 declare function astCheckConvNonPtrToPtr _
 	( _
 		byval to_dtype as integer, _

src/compiler/error.bas

@@ -398,7 +398,8 @@ declare function hMakeParamDesc _
 		@"CONST used on constructor (not needed)", _
 		@"CONST used on destructor (not needed)", _
 		@"Byref function result not set", _
-		@"Function result assignment outside of the function" _
+		@"Function result assignment outside of the function", _
+		@"Type mismatch in byref function result assignment" _
 	}
 
 

src/compiler/error.bi

@@ -320,6 +320,7 @@ enum FB_ERRMSG
 	FB_ERRMSG_CONSTDTOR
 	FB_ERRMSG_NOBYREFFUNCTIONRESULT
 	FB_ERRMSG_RESULTASSIGNOUTSIDEFUNCTION
+	FB_ERRMSG_TYPEMISMATCHINBYREFRESULTASSIGN
 
 	FB_ERRMSGS
 end enum

src/compiler/parser-proccall.bas

@@ -91,18 +91,20 @@ function cAssignFunctResult( byval is_return as integer ) as integer
 				errReport( FB_ERRMSG_INVALIDREFERENCETOLOCAL )
 			end if
 
-			'' BYREF AS STRING and expression is a string literal?
-			if( (symbGetType( parser.currproc ) = FB_DATATYPE_STRING) and _
-			    (astGetStrLitSymbol( rhs ) <> NULL) ) then
-				'' Cannot be allowed, since a string literal is not
-				'' an FBSTRING descriptor...
-				errReport( FB_ERRMSG_INVALIDDATATYPES )
+			'' Even though we'll only assign a pointer (it's byref afterall),
+			'' check the types as if it was a direct assignment. Advantages:
+			''  * errors instead of warnings
+			''  * nicer error messages such as "type mismatch" instead of
+			''    "suspicious pointer assignment" warnings (which don't make sense
+			''    since the pointer basically is an implementation detail)
+			if( astCheckByrefAssign( parser.currproc->typ, parser.currproc->subtype, rhs ) = FALSE ) then
+				errReport( FB_ERRMSG_TYPEMISMATCHINBYREFRESULTASSIGN )
 				astDelTree( rhs )
-				rhs = NULL
-			else
-				'' Implicit addrof due to BYREF
-				rhs = astNewADDROF( rhs )
+				rhs = astNewCONSTz( parser.currproc->typ, parser.currproc->subtype )
 			end if
+
+			'' Implicit addrof due to BYREF
+			rhs = astNewADDROF( rhs )
 		end if
 	else
 		rhs = cExpression( )

src/compiler/symb-data.bas

@@ -491,6 +491,13 @@ function closestType _
 
 end function
 
+'' Determine whether two types are compatible, for an assignment like this:
+''    l = r
+'' where l could be a reference or a variable (parameters, function results,
+'' variables...) and r is the source (e.g. the argument in case of parameters).
+''  result = 0  =>  incompatible
+''  otherwise   =>  compatible, and the result is a matching score
+''                  (FB_OVLPROC_FULLMATCH etc.)
 function typeCalcMatch _
 	( _
 		byval ldtype as integer, _

tests/functions/return-byref-type-match-1.bas

@@ -0,0 +1,6 @@
+' TEST_MODE : COMPILE_ONLY_OK
+
+function f() byref as const integer
+	static i as const integer = 0
+	function = i
+end function

tests/functions/return-byref-type-match-2.bas

@@ -0,0 +1,6 @@
+' TEST_MODE : COMPILE_ONLY_OK
+
+function f() byref as const integer
+	static i as integer = 0
+	function = i
+end function

tests/functions/return-byref-type-mismatch-1.bas

@@ -0,0 +1,6 @@
+' TEST_MODE : COMPILE_ONLY_FAIL
+
+function f() byref as integer
+	static i as double
+	function = i
+end function