fbc: string*n intialization

fixed: Copy back string*N passed as argument to zstring parameters

- arguments of STRING*N type are copied to a temporary variable when passed to BYVAL ZSTRING PTR and BYREF ZSTRING parameters (since there is no expectaction that STRING*N will have it's own terminating null character) and copied back after the call
- tests added

fixed: Initialize UDT fields with spaces

changed: Improve on initialization of Types and Unions:

- unions containing STRING*N fields will initialize to spaces if the first field
  is a STRING*N type, otherwise will initialize to zero
- internal: types containing only STRING*N fields or child types containing
  only STRING*N fields will initialize to spaces using a memory fill operation,
  and where other data types are present will induce the creation of an
  implicit constructor

Internal:

Added FB_UDTOPT_HASZEROEDFIELD and FB_UDTOPT_HASFILLEDFIELD to track
fields that can be zeroed or filled during the composition of a UDT.
This allows making decisions about how a field can be initialized.
Arguably not the most efficient method to track this information and
perhaps later we should add to FBS_STRUCT to make decision if it
is more efficient to generate an implicit constructor or inline
assignments even for non-zero and initialized fields.

changelog.txt

@@ -12,10 +12,12 @@ Version 1.20.0
 - rtlib: optimize string concatenation reallocations: do not reallocate to smaller memory buffer if the contents are to be preserved
 - limit WSTRING*N, ZSTRING*N, STRING*N types to 2^31-1 and show an 'invalid size' error if the size given is less than 1 or greater than the limit
 - STRING*N occupies N bytes in memory, pads the variable / field with spaces, and does not automatically append a terminating null character
-- STRING*N variables and fields are initialized to spaces by default
+- STRING*N variables and type fields are initialized to spaces by default
+- internal: types containing only STRING*N fields or child types containing only STRING*N fields will initialize to spaces using a memory fill operation, and where other data types are present will induce the creation of an implicit constructor
+- unions containing STRING*N fields will initialize to spaces if the first field is a STRING*N type, otherwise will initialize to zero
 - LEN(STRING*N) = SIZEOF(STRING*N) due to padding and no null terminator
 - STRING*N/SWAP: swap will pad values with spaces where one of the arguments is of the STRING*N data type
-- STRING*N is copied to a temporary variable when passed to ZSTRING PTR parameters since there is no expectaction that STRING*N will have it's own terminating null character
+- arguments of STRING*N type are copied to a temporary variable when passed to BYVAL ZSTRING PTR and BYREF ZSTRING parameters (since there is no expectaction that STRING*N will have it's own terminating null character) and copied back after the call
 
 [added]
 - x86_64: optimize SHL MOD INTDIV to use 32-bit operation when result will be converted to long/ulong

src/compiler/ast-node-arg.bas

@@ -944,10 +944,10 @@ private function hCheckParam _
 				'' if it's a wstring param, convert..
 				n->l = rtlToStr( n->l, FALSE )
 			case FB_DATATYPE_FIXSTR
-				'' if it's a fxied length string, make a copy
+				'' if it's a fixed length string, make a copy
 				'' because STRING*N is not guaranteed to have
 				'' a null terminator
-				n->l = hAllocTempString( parent, n->l, FALSE )
+				n->l = hAllocTempString( parent, n->l, TRUE )
 			end select
 
 		'' wstring ptr / wstring?

src/compiler/ast-node-decl.bas

@@ -79,10 +79,21 @@ private function hDefaultInit( byval sym as FBSYMBOL ptr ) as ASTNODE ptr
 		exit function
 	end if
 
+
+	select case symbGetType( sym )
+
 	'' fixed string? initialize to spaces
-	if( symbGetType( sym ) = FB_DATATYPE_FIXSTR ) then
+	case FB_DATATYPE_FIXSTR
 		fillchar = 32
-	end if
+
+	'' UDT containing only fixed length string?
+	'' optimize to a memory fill instead of implicit constructor
+	case FB_DATATYPE_STRUCT
+		if( symbGetUDTHasFilledField( sym->subtype ) ) then
+			fillchar = 32
+		end if
+
+	end select
 
 	function = astNewMEM( AST_OP_MEMFILL, astNewVAR( sym ), _
 	                      astNewCONSTi( symbGetSizeOf( sym ) _

src/compiler/ast-node-proc.bas

@@ -996,7 +996,8 @@ private function hCallFieldCtor _
 	else
 		function = astNewMEM( AST_OP_MEMFILL, _
 		                      astBuildVarField( this_, fld ), _
-		                      astNewCONSTi( symbGetRealSize( fld ) ) )
+		                      astNewCONSTi( symbGetRealSize( fld ) ), _
+		                      0, iif( (symbGetType( fld ) = FB_DATATYPE_FIXSTR), 32, 0 ) )
 	end if
 end function
 

src/compiler/symb-struct.bas

@@ -472,6 +472,7 @@ function symbAddField _
 			symbSetUDTHasDtorField( parent )
 			symbSetUDTHasPtrField( parent )
 		end if
+		symbSetUDTHasZeroedField( parent )
 	else
 		select case( typeGetDtAndPtrOnly( dtype ) )
 		'' var-len string fields? must add a ctor, copyctor and dtor
@@ -484,6 +485,7 @@ function symbAddField _
 				symbSetUDTHasDtorField( parent )
 				symbSetUDTHasPtrField( parent )
 			end if
+			symbSetUDTHasZeroedField( parent )
 
 		'' struct with a ctor or dtor? must add a ctor or dtor too
 		case FB_DATATYPE_STRUCT
@@ -493,6 +495,18 @@ function symbAddField _
 				symbSetUDTHasPtrField( base_parent )
 			end if
 
+			'' Let the FB_UDTOPT_HASFILLEDFIELD flag propagate up to the
+			'' parent if this field has it.
+			if( symbGetUDTHasFilledField( subtype ) ) then
+				symbSetUDTHasFilledField( base_parent )
+			end if
+
+			'' Let the FB_UDTOPT_HASZEROEDFIELD flag propagate up to the
+			'' parent if this field has it.
+			if( symbGetUDTHasZeroedField( subtype ) ) then
+				symbSetUDTHasZeroedField( base_parent )
+			end if
+
 			if( symbGetCompCtorHead( subtype ) ) then
 				'' not allowed inside unions or anonymous nested structs/unions
 				if( symbGetUDTIsUnionOrAnon( parent ) ) then
@@ -511,6 +525,28 @@ function symbAddField _
 				end if
 			end if
 
+		case FB_DATATYPE_FIXSTR
+			'' only the first field in a UNION can decide if it will be zeroed or filled
+			if( symbGetUDTIsUnion( parent ) ) then
+				if( (symbGetUDTHasFilledField( parent ) = FALSE) andalso _
+				    (symbGetUDTHasZeroedField( parent ) = FALSE) ) then
+					symbSetUDTHasFilledField( parent )
+				end if
+			else
+				symbSetUDTHasFilledField( parent )
+			end if
+
+		case else
+			'' only the first field in a UNION can decide if it will be zeroed or filled
+			if( symbGetUDTIsUnion( parent ) ) then
+				if( (symbGetUDTHasFilledField( parent ) = FALSE) andalso _
+				    (symbGetUDTHasZeroedField( parent ) = FALSE) ) then
+					symbSetUDTHasZeroedField( parent )
+				end if
+			else
+				symbSetUDTHasZeroedField( parent )
+			end if
+
 		end select
 	end if
 
@@ -915,6 +951,15 @@ sub symbStructEnd _
 		end if
 	end if
 
+	'' Zeroed fields and Filled fields are incompatible since
+	'' we expect to only use a single memory zero/fill operation
+	if( symbGetUDTHasZeroedField( sym ) andalso symbGetUDTHasFilledField( sym ) ) then
+		'' handle through implicit constructor
+		symbSetUDTHasInitedField( sym )
+		sym->udt.options and= not FB_UDTOPT_HASFILLEDFIELD
+	end if
+
+
 	'' Declare implicit members (but don't implement them yet)
 	symbUdtDeclareDefaultMembers( defaultmembers, sym, mode )
 

src/compiler/symb.bi

@@ -493,6 +493,8 @@ enum FB_UDTOPT
 	FB_UDTOPT_ISWSTRING         = &h00010000
 	FB_UDTOPT_ISZSTRING         = &h00020000
 	FB_UDTOPT_HASNESTED         = &h00040000
+	FB_UDTOPT_HASZEROEDFIELD    = &h00080000
+	FB_UDTOPT_HASFILLEDFIELD    = &h00100000
 	FB_UDTOPT_VALISTTYPEMASK    = &h00f00000
 end enum
 
@@ -2389,6 +2391,12 @@ declare function symbGetWstrLength( byval sym as FBSYMBOL ptr ) as longint
 #define symbSetUdtHasNested( s )   (s)->udt.options or= FB_UDTOPT_HASNESTED
 #define symbGetUdtHasNested( s ) (((s)->udt.options and FB_UDTOPT_HASNESTED) <> 0)
 
+#define symbSetUDTHasZeroedField( s ) (s)->udt.options or= FB_UDTOPT_HASZEROEDFIELD
+#define symbGetUDTHasZeroedField( s ) (((s)->udt.options and FB_UDTOPT_HASZEROEDFIELD) <> 0)
+
+#define symbSetUDTHasFilledField( s ) (s)->udt.options or= FB_UDTOPT_HASFILLEDFIELD
+#define symbGetUDTHasFilledField( s ) (((s)->udt.options and FB_UDTOPT_HASFILLEDFIELD) <> 0)
+
 #define symbGetUDTIsUnionOrAnon(s) (((s)->udt.options and (FB_UDTOPT_ISUNION or FB_UDTOPT_ISANON)) <> 0)
 
 #define symbGetUDTAlign(s) s->udt.align

tests/dim/string_init.bas

@@ -12,8 +12,19 @@ SUITE( fbc_tests.dim_.string_init )
 	TEST( fixedLength )
 		dim as string * 5 s1 = "test"
 		dim s2 as string * 5 = "test"
+
+		dim s3 as string * 5 = ""
+		dim as string * 5 s4 = ""
+
+		dim s5 as string * 5
+		dim as string * 5 s6
+
 		CU_ASSERT( s1 = "test " )
 		CU_ASSERT( s2 = "test " )
+		CU_ASSERT( s3 = space( 5 ) )
+		CU_ASSERT( s4 = space( 5 ) )
+		CU_ASSERT( s5 = space( 5 ) )
+		CU_ASSERT( s6 = space( 5 ) )
 	END_TEST
 
 END_SUITE

tests/functions/param-string-copyback.bas

@@ -43,6 +43,39 @@ SUITE( fbc_tests.functions.param_string_copyback )
 		END_TEST
 	end namespace
 
+	namespace ns1z
+		sub chk_byval_zstring_copyback( byval s as zstring ptr )
+			*s = ucase( *s )
+		end sub
+
+		sub chk_byref_zstring_copyback( byref s as zstring )
+			s = ucase( s )
+		end sub
+
+		TEST( simple_zstring )
+			dim s as string
+			dim f as string * 3
+			dim z as zstring * 3+1
+			dim w as wstring * 3+1
+			dim pz as zstring ptr = @z
+			dim pw as wstring ptr = @w
+
+			s = "var" : chk_byval_zstring_copyback( s   ) : CU_ASSERT( s = "VAR" ) '' copyback
+			f = "fix" : chk_byval_zstring_copyback( f   ) : CU_ASSERT( f = "FIX" ) '' copyback
+			z = "zer" : chk_byval_zstring_copyback( z   ) : CU_ASSERT( z = "ZER" ) '' copyback
+			z = "zer" : chk_byval_zstring_copyback( *pz ) : CU_ASSERT( z = "ZER" ) '' copyback
+			w = "foo" : chk_byval_zstring_copyback( w   ) : CU_ASSERT( w = "foo" ) '' no copyback
+			w = "foo" : chk_byval_zstring_copyback( *pw ) : CU_ASSERT( w = "foo" ) '' no copyback
+
+			s = "var" : chk_byref_zstring_copyback( s   ) : CU_ASSERT( s = "VAR" ) '' copyback
+			f = "fix" : chk_byref_zstring_copyback( f   ) : CU_ASSERT( f = "FIX" ) '' copyback
+			z = "zer" : chk_byref_zstring_copyback( z   ) : CU_ASSERT( z = "ZER" ) '' copyback
+			z = "zer" : chk_byref_zstring_copyback( *pz ) : CU_ASSERT( z = "ZER" ) '' copyback
+			w = "foo" : chk_byref_zstring_copyback( w   ) : CU_ASSERT( w = "foo" ) '' no copyback
+			w = "foo" : chk_byref_zstring_copyback( *pw ) : CU_ASSERT( w = "foo" ) '' no copyback
+		END_TEST
+	end namespace
+
 	namespace ns2
 		sub appendChars( byref s as string, byval n as integer )
 			CU_ASSERT( rtrim(s) = "a" )

tests/quirk/len-sizeof.bas

@@ -28,9 +28,6 @@ SUITE( fbc_tests.quirk.len_sizeof )
 		CU_ASSERT( len( string( 3, "a" ) ) = 3 )  '' Relies on disambiguation from len(string) via the '(' in 'string('
 		CU_ASSERT( len( (string( 3, "a" )) ) = 3 ) '' Not ambiguous
 
-		'' FIXSTRs are somewhat of an exception though,
-		'' len() returns the sizeof()-1 (-1 for the implicit null terminator),
-		'' i.e. the N from STRING * N, not the length of the stored string data.
 		dim fstr as string * 31
 		CU_ASSERT(    len( fstr ) = 31 )
 		CU_ASSERT( sizeof( fstr ) = 31 )
@@ -49,14 +46,21 @@ SUITE( fbc_tests.quirk.len_sizeof )
 		CU_ASSERT(    len( wstr( "1234567" ) ) = 7 )
 		CU_ASSERT( sizeof( wstr( "1234567" ) ) = 8 * sizeof( wstring ) )
 
-		type UDT
+		'' mixing of zstring*N and string*N in the same type
+		'' induces the creation of an implicit constructor
+		'' so we can't scope one UDT here that contains both
+
+		type UDTZ
 			zstr as zstring * len( STRCONST ) + 1
-			fstr as  string * len( STRCONST )
 		end type
-
-		dim x as UDT
+		dim x as UDTZ
 		CU_ASSERT( sizeof( x.zstr ) = len( STRCONST ) + 1 )
-		CU_ASSERT(    len( x.fstr ) = len( STRCONST )     )
+
+		type UDTF
+			fstr as  string * len( STRCONST )
+		 end type
+		dim y as UDTF
+		CU_ASSERT(    len( y.fstr ) = len( STRCONST )     )
 	END_TEST
 
 	namespace ns

tests/structs/field-init.bas

@@ -35,6 +35,68 @@ SUITE( fbc_tests.structs.field_init )
 		END_TEST
 	END_TEST_GROUP
 
+	TEST_GROUP( fixedstrings )
+		type T1
+			as string * 10 a
+		end type
+
+		type T2
+			as string * 10 a = ""
+		end type
+
+		type T3
+			as string * 10 a = "abc"
+		end type
+
+		type T4
+			as zstring * 2  a1
+			as string * 3   a2
+			as zstring * 4  a3
+			as string * 5   a4
+			as zstring * 6  a5
+		end type
+
+		union U1
+			c(0 to 9) as ubyte
+			f as string * 10
+		end union
+
+		union U2
+			f as string * 10
+			c(0 to 9) as ubyte
+		end union
+
+		TEST( default )
+			dim x1 as T1
+			CU_ASSERT( x1.a = space( 10 ) )
+			print asc( x1.a, 0 )
+
+			dim x2 as T2
+			CU_ASSERT( x2.a = space( 10 ) )
+
+			dim x3 as T3
+			CU_ASSERT( x3.a = "abc       " )
+
+			dim x4 as T4
+			CU_ASSERT( x4.a1 = "" )
+			CU_ASSERT( x4.a2 = space(3) )
+			CU_ASSERT( x4.a3 = "" )
+			CU_ASSERT( x4.a4 = space(5) )
+			CU_ASSERT( x4.a5 = "" )
+
+			dim x5 as U1 '' first field is byte array
+			CU_ASSERT( x5.c(0) = 0 )
+			CU_ASSERT( x5.c(9) = 0 )
+			CU_ASSERT( x5.f = string(10, 0) )
+
+			dim x6 as U2 '' first field is fixed length string
+			CU_ASSERT( x6.f = string(10, 32) )
+			CU_ASSERT( x6.c(0) = 32 )
+			CU_ASSERT( x6.c(9) = 32 )
+
+		END_TEST
+	END_TEST_GROUP
+
 	'' Complex field init
 	TEST_GROUP( complex )
 		type A
@@ -237,9 +299,9 @@ SUITE( fbc_tests.structs.field_init )
 
 		type foo field = 1
 			pad as integer
-			f1(0 to 2) as ubyte 
-			f2(0 to 2) as ushort 
-			f3(0 to 1) as uinteger 
+			f1(0 to 2) as ubyte
+			f2(0 to 2) as ushort
+			f3(0 to 1) as uinteger
 		end type
 
 		type bar field = 1