cgen: minimise sizeof(EmptyStruct) to 0 for gcc/clang and to 1 for tcc/msvc, by changing EMPTY_STRUCT_DECLARATION and EMPTY_STRUCT_INITIALIZATION (#16733)

Author: spytheman

vlib/arrays/arrays_test.v

@@ -355,7 +355,7 @@ fn test_array_append_empty_struct() {
 	assert (XYZ{} in names) == true
 
 	// test fixed array
-	array := [XYZ{}]
+	array := [XYZ{}]!
 	assert (XYZ{} in names) == true
 }
 

vlib/builtin/array.v

@@ -47,9 +47,13 @@ fn __new_array_with_default(mylen int, cap int, elm_size int, val voidptr) array
 		len: mylen
 		cap: cap_
 	}
+	// x := []EmptyStruct{cap:5} ; for clang/gcc with -gc none,
+	//    -> sizeof(EmptyStruct) == 0 -> elm_size == 0
+	//    -> total_size == 0 -> malloc(0) -> panic;
+	//    to avoid it, just allocate a single byte
 	total_size := u64(cap_) * u64(elm_size)
 	if cap_ > 0 && mylen == 0 {
-		arr.data = unsafe { malloc(total_size) }
+		arr.data = unsafe { malloc(__at_least_one(total_size)) }
 	} else {
 		arr.data = vcalloc(total_size)
 	}
@@ -78,7 +82,7 @@ fn __new_array_with_array_default(mylen int, cap int, elm_size int, val array, d
 	cap_ := if cap < mylen { mylen } else { cap }
 	mut arr := array{
 		element_size: elm_size
-		data: unsafe { malloc(u64(cap_) * u64(elm_size)) }
+		data: unsafe { malloc(__at_least_one(u64(cap_) * u64(elm_size))) }
 		len: mylen
 		cap: cap_
 	}
@@ -99,7 +103,7 @@ fn __new_array_with_map_default(mylen int, cap int, elm_size int, val map) array
 	cap_ := if cap < mylen { mylen } else { cap }
 	mut arr := array{
 		element_size: elm_size
-		data: unsafe { malloc(u64(cap_) * u64(elm_size)) }
+		data: unsafe { malloc(__at_least_one(u64(cap_) * u64(elm_size))) }
 		len: mylen
 		cap: cap_
 	}
@@ -156,7 +160,7 @@ fn (mut a array) ensure_cap(required int) {
 		cap *= 2
 	}
 	new_size := u64(cap) * u64(a.element_size)
-	new_data := unsafe { malloc(new_size) }
+	new_data := unsafe { malloc(__at_least_one(new_size)) }
 	if a.data != unsafe { nil } {
 		unsafe { vmemcpy(new_data, a.data, u64(a.len) * u64(a.element_size)) }
 		// TODO: the old data may be leaked when no GC is used (ref-counting?)
@@ -601,13 +605,9 @@ pub fn (a &array) clone() array {
 // recursively clone given array - `unsafe` when called directly because depth is not checked
 [unsafe]
 pub fn (a &array) clone_to_depth(depth int) array {
-	mut size := u64(a.cap) * u64(a.element_size)
-	if size == 0 {
-		size++
-	}
 	mut arr := array{
 		element_size: a.element_size
-		data: vcalloc(size)
+		data: vcalloc(u64(a.cap) * u64(a.element_size))
 		len: a.len
 		cap: a.cap
 	}

vlib/builtin/builtin.c.v

@@ -399,6 +399,16 @@ pub fn malloc_noscan(n isize) &u8 {
 	return res
 }
 
+[inline]
+fn __at_least_one(how_many u64) u64 {
+	// handle the case for allocating memory for empty structs, which have sizeof(EmptyStruct) == 0
+	// in this case, just allocate a single byte, avoiding the panic for malloc(0)
+	if how_many == 0 {
+		return 1
+	}
+	return how_many
+}
+
 // malloc_uncollectable dynamically allocates a `n` bytes block of memory
 // on the heap, which will NOT be garbage-collected (but its contents will).
 [unsafe]

vlib/builtin/map.v

@@ -100,8 +100,8 @@ fn new_dense_array(key_bytes int, value_bytes int) DenseArray {
 		len: 0
 		deletes: 0
 		all_deleted: 0
-		keys: unsafe { malloc(cap * key_bytes) }
-		values: unsafe { malloc(cap * value_bytes) }
+		keys: unsafe { malloc(__at_least_one(u64(cap) * u64(key_bytes))) }
+		values: unsafe { malloc(__at_least_one(u64(cap) * u64(value_bytes))) }
 	}
 }
 

vlib/builtin/map_d_gcboehm_opt.v

@@ -6,28 +6,26 @@
 
 module builtin
 
+[inline]
+fn __malloc_at_least_one(how_many_bytes u64, noscan bool) &u8 {
+	if noscan {
+		return unsafe { malloc_noscan(__at_least_one(how_many_bytes)) }
+	}
+	return unsafe { malloc(__at_least_one(how_many_bytes)) }
+}
+
 [inline]
 fn new_dense_array_noscan(key_bytes int, key_noscan bool, value_bytes int, value_noscan bool) DenseArray {
 	cap := 8
-	keys := if key_noscan {
-		unsafe { malloc_noscan(cap * key_bytes) }
-	} else {
-		unsafe { malloc(cap * key_bytes) }
-	}
-	values := if value_noscan {
-		unsafe { malloc_noscan(cap * value_bytes) }
-	} else {
-		unsafe { malloc(cap * value_bytes) }
-	}
 	return DenseArray{
 		key_bytes: key_bytes
 		value_bytes: value_bytes
 		cap: cap
 		len: 0
 		deletes: 0
 		all_deleted: 0
-		keys: keys
-		values: values
+		keys: __malloc_at_least_one(u64(cap) * u64(key_bytes), key_noscan)
+		values: __malloc_at_least_one(u64(cap) * u64(value_bytes), value_noscan)
 	}
 }
 

vlib/v/gen/c/array.v

@@ -34,8 +34,6 @@ fn (mut g Gen) array_init(node ast.ArrayInit, var_name string) {
 		noscan := g.check_noscan(elem_type.typ)
 		if elem_type.unaliased_sym.kind == .function {
 			g.write('new_array_from_c_array(${len}, ${len}, sizeof(voidptr), _MOV((voidptr[${len}]){')
-		} else if g.is_empty_struct(elem_type) {
-			g.write('new_array_from_c_array${noscan}(${len}, ${len}, sizeof(voidptr), _MOV((${elem_styp}[${len}]){')
 		} else {
 			g.write('new_array_from_c_array${noscan}(${len}, ${len}, sizeof(${elem_styp}), _MOV((${elem_styp}[${len}]){')
 		}
@@ -216,7 +214,7 @@ fn (mut g Gen) array_init_with_fields(node ast.ArrayInit, elem_type Type, is_amp
 		} else {
 			g.write('0, ')
 		}
-		if elem_type.unaliased_sym.kind == .function || g.is_empty_struct(elem_type) {
+		if elem_type.unaliased_sym.kind == .function {
 			g.write('sizeof(voidptr), ')
 		} else {
 			g.write('sizeof(${elem_styp}), ')
@@ -292,7 +290,7 @@ fn (mut g Gen) array_init_with_fields(node ast.ArrayInit, elem_type Type, is_amp
 	} else {
 		g.write('0, ')
 	}
-	if elem_type.unaliased_sym.kind == .function || g.is_empty_struct(elem_type) {
+	if elem_type.unaliased_sym.kind == .function {
 		g.write('sizeof(voidptr), ')
 	} else {
 		g.write('sizeof(${elem_styp}), ')

vlib/v/gen/c/cheaders.v

@@ -253,8 +253,8 @@ static void* __closure_create(void* fn, void* data) {
 
 const c_common_macros = '
 #define EMPTY_VARG_INITIALIZATION 0
-#define EMPTY_STRUCT_INITIALIZATION 0
-#define EMPTY_STRUCT_DECLARATION voidptr _dummy_pad
+#define EMPTY_STRUCT_DECLARATION
+#define EMPTY_STRUCT_INITIALIZATION
 // Due to a tcc bug, the length of an array needs to be specified, but GCC crashes if it is...
 #define EMPTY_ARRAY_OF_ELEMS(x,n) (x[])
 #define TCCSKIP(x) x
@@ -312,9 +312,12 @@ const c_common_macros = '
 #ifdef __clang__
 	#undef __V_GCC__
 #endif
+
 #ifdef _MSC_VER
 	#undef __V_GCC__
+	#undef EMPTY_STRUCT_DECLARATION
 	#undef EMPTY_STRUCT_INITIALIZATION
+	#define EMPTY_STRUCT_DECLARATION unsigned char _dummy_pad
 	#define EMPTY_STRUCT_INITIALIZATION 0
 #endif
 
@@ -332,7 +335,9 @@ const c_common_macros = '
 #ifdef __TINYC__
 	#define _Atomic volatile
 	#undef EMPTY_STRUCT_DECLARATION
-	#define EMPTY_STRUCT_DECLARATION voidptr _dummy_pad
+	#undef EMPTY_STRUCT_INITIALIZATION
+	#define EMPTY_STRUCT_DECLARATION unsigned char _dummy_pad
+	#define EMPTY_STRUCT_INITIALIZATION 0
 	#undef EMPTY_ARRAY_OF_ELEMS
 	#define EMPTY_ARRAY_OF_ELEMS(x,n) (x[n])
 	#undef __NOINLINE
@@ -594,10 +599,7 @@ voidptr memdup(voidptr src, int sz);
 		#define _Atomic volatile
 
 		// MSVC cannot parse some things properly
-		#undef EMPTY_STRUCT_DECLARATION
 		#undef OPTION_CAST
-
-		#define EMPTY_STRUCT_DECLARATION voidptr _dummy_pad
 		#define OPTION_CAST(x)
 		#undef __NOINLINE
 		#undef __IRQHANDLER

vlib/v/tests/arrays_and_maps_of_empty_structs_should_work_test.v

@@ -0,0 +1,40 @@
+struct EmptyStruct {}
+
+fn test_array_append_empty_struct() {
+	size_of_empty_struct := dump(sizeof(EmptyStruct))
+	assert size_of_empty_struct <= 1
+	mut names := []EmptyStruct{cap: 2}
+	names << EmptyStruct{}
+	dump(names)
+	assert (EmptyStruct{} in names) == true
+	//
+	mut fa := [EmptyStruct{}, EmptyStruct{}]!
+	assert fa.len == 2
+	dump(fa[0])
+	fa[0] = EmptyStruct{}
+	dump(fa[0])
+	assert fa[0] == EmptyStruct{}
+	assert fa[1] == EmptyStruct{}
+}
+
+fn test_map_of_int_set_empty_struct() {
+	mut names := map[int]EmptyStruct{}
+	names[10] = EmptyStruct{}
+	names[99] = EmptyStruct{}
+	dump(names)
+	assert 10 in names
+	assert 99 in names
+	assert names[10] == EmptyStruct{}
+	assert names[99] == EmptyStruct{}
+}
+
+fn test_map_of_string_set_empty_struct() {
+	mut names := map[string]EmptyStruct{}
+	names['ab'] = EmptyStruct{}
+	names['cd'] = EmptyStruct{}
+	dump(names)
+	assert 'ab' in names
+	assert 'cd' in names
+	assert names['ab'] == EmptyStruct{}
+	assert names['cd'] == EmptyStruct{}
+}

vlib/v/tests/empty_struct_test.v

@@ -0,0 +1,53 @@
+struct Z0 {}
+
+struct Z1 {
+	padding1 char
+}
+
+struct Z2 {
+	padding1 char
+	padding2 char
+}
+
+struct Z3 {
+	padding1 char
+	padding2 char
+	padding3 char
+}
+
+struct Z4 {
+	padding1 char
+	padding2 char
+	padding3 char
+	padding4 char
+}
+
+fn test_struct_sizes() {
+	assert dump(sizeof(Z0)) <= 1 // valid for all
+	$if tinyc {
+		// TCC has no problems with 0 sized structs in almost cases,
+		// except when they are used in fixed arrays, or their address is taken,
+		// in which case, it produces a compilation error. To avoid it, for it
+		// empty structs are 1 byte in size.
+		assert dump(sizeof(Z0)) == 1
+	}
+	$if msvc {
+		// MSVC seems to have no way at all to have empty structs in C mode. It produces the following error:
+		// `error c2016: C requires that a struct or union have at least one member`.
+		// Note that MSVC allows empty structs in C++ mode, but that has other restrictions,
+		// and is not suitable for the generated code of most V programs. Besides, even in C++  mode, the size of
+		// an empty struct is still 1, not 0.
+		// For that reason, empty structs are 1 byte in size for MSVC too.
+		assert dump(sizeof(Z0)) == 1
+	}
+	$if clang {
+		assert dump(sizeof(Z0)) == 0
+	}
+	$if gcc {
+		assert dump(sizeof(Z0)) == 0
+	}
+	assert dump(sizeof(Z1)) < sizeof(Z2)
+	assert dump(sizeof(Z2)) < sizeof(Z3)
+	assert dump(sizeof(Z3)) < sizeof(Z4)
+	assert dump(sizeof(Z4)) == 4
+}