~sircmpwn/hare-dev

This thread contains a patchset. You're looking at the original emails, but you may wish to use the patch review UI. Review patch
3 2

[PATCH harec] fix branching expression result type calculation

Details
Message ID
<20250117030718.3160-1-bgs@turminal.net>
Sender timestamp
1737086652
DKIM signature
pass
Download raw message
Patch: +109 -50
This is essentially the same change as 56f9fa50, but for match, if and
compound expressions instead of switch. The reasoning behind this change
is provided in the accompanying spec commit.

This commit makes harec comply with the new spec and ensures the change
does not introduce problems in gen.

Breaking-Change: language
Signed-off-by: Bor Grošelj Simić <bgs@turminal.net>
---
This implements the spec change at
https://lists.sr.ht/~sircmpwn/hare-dev/patches/56767

 src/check.c            | 121 +++++++++++++++++++++++++----------------
 src/gen.c              |  18 ++++++
 tests/18-match.ha      |   6 ++
 tests/20-if.ha         |   7 ++-
 tests/26-regression.ha |   7 ++-
 5 files changed, 109 insertions(+), 50 deletions(-)

diff --git a/src/check.c b/src/check.c
index dd91dd88..8d2dbfe5 100644
--- a/src/check.c
+++ b/src/check.c
@@ -1759,10 +1759,31 @@ check_expr_compound(struct context *ctx,
		check_expression(ctx, yexpr, lexpr, NULL);
		list->next->expr = lexpr;
	}
	expr->result = type_store_reduce_result(ctx, aexpr->loc,
			scope->results);
	if (hint) {
		expr->result = hint;
	} else {
		expr->result = type_store_reduce_result(
			ctx, aexpr->loc, scope->results);
		if (expr->result == NULL) {
			error(ctx, aexpr->loc, expr,
				"Invalid result type (dangling or ambiguous null)");
			return;
		}
	}

	for (struct yield *yield = scope->yields; yield;) {
		if (hint && !type_is_assignable(ctx, hint,
				(*yield->expression)->result)) {
			char *yieldtypename = gen_typename((*yield->expression)->result);
			char *hinttypename = gen_typename(hint);
			error(ctx, expr->loc, expr,
				"Branch type (%s) is not assignable to hint (%s)",
				yieldtypename, hinttypename);
			free(yieldtypename);
			free(hinttypename);
			return;

		}
		*yield->expression = lower_implicit_cast(ctx, expr->result,
			*yield->expression);

@@ -2342,14 +2363,29 @@ check_expr_if(struct context *ctx,
	} else {
		false_branch->type = EXPR_LITERAL;
		false_branch->result = &builtin_type_void;
		false_branch->loc = expr->loc;
	}
	const struct type *fresult = false_branch->result;
	if (hint && type_is_assignable(ctx, hint, true_branch->result)
			&& type_is_assignable(ctx, hint, fresult)) {
	if (hint) {
		struct expression *branch = NULL;
		if (!type_is_assignable(ctx, hint, true_branch->result)) {
			branch = true_branch;
		} else if (!type_is_assignable(ctx, hint, false_branch->result)) {
			branch = false_branch;
		}
		if (branch) {
			char *branchtypename = gen_typename(branch->result);
			char *hinttypename = gen_typename(hint);
			error(ctx, branch->loc, expr,
				"Branch type (%s) is not assignable to hint (%s)",
				branchtypename, hinttypename);
			free(branchtypename);
			free(hinttypename);
			return;
		}
		expr->result = hint;
	} else {
		struct type_tagged_union _tags = {
			.type = fresult,
			.type = false_branch->result,
			.next = NULL,
		}, tags = {
			.type = true_branch->result,
@@ -2466,10 +2502,7 @@ check_expr_match(struct context *ctx,
		return;
	}

	struct type_tagged_union result_type = {0};
	struct type_tagged_union *tagged = &result_type,
		**next_tag = &tagged->next;

	struct type_tagged_union *tagged = NULL, *result = NULL;
	struct match_case **next = &expr->match.cases, *_case = NULL;
	for (struct ast_match_case *acase = aexpr->match.cases;
			acase; acase = acase->next) {
@@ -2535,50 +2568,44 @@ check_expr_match(struct context *ctx,
			scope_pop(&ctx->scope);
		}

		if (expr->result == NULL) {
			expr->result = _case->value->result;
			tagged->type = expr->result;
		} else if (expr->result != _case->value->result) {
			tagged = *next_tag =
				xcalloc(1, sizeof(struct type_tagged_union));
			next_tag = &tagged->next;
			tagged->type = _case->value->result;
		if (tagged == NULL) {
			result = tagged = xcalloc(1, sizeof *tagged);
		} else if (tagged->type && tagged->type != _case->value->result) {
			tagged = tagged->next = xcalloc(1, sizeof *tagged);
		}
		tagged->type = _case->value->result;
	}

	if (result_type.next) {
		if (hint) {
			expr->result = hint;
		} else {
			expr->result = type_store_reduce_result(
				ctx, aexpr->loc, &result_type);
			if (expr->result == NULL) {
				error(ctx, aexpr->loc, expr,
					"Invalid result type (dangling or ambiguous null)");
				return;
			}
	if (hint) {
		expr->result = hint;
	} else {
		expr->result = type_store_reduce_result(
			ctx, aexpr->loc, result);
		if (expr->result == NULL) {
			error(ctx, aexpr->loc, expr,
				"Invalid result type (dangling or ambiguous null)");
			return;
		}
	}

		struct match_case *_case = expr->match.cases;
		struct ast_match_case *acase = aexpr->match.cases;
		while (_case) {
			if (hint && !type_is_assignable(ctx, hint, _case->value->result)) {
				error(ctx, acase->exprs.expr->loc, expr,
					"Match case is not assignable to result type");
				return;
			}
			_case->value = lower_implicit_cast(ctx, 
				expr->result, _case->value);
			_case = _case->next;
			acase = acase->next;
	_case = expr->match.cases;
	struct ast_match_case *acase = aexpr->match.cases;
	while (_case) {
		if (hint && !type_is_assignable(ctx, hint, _case->value->result)) {
			error(ctx, acase->exprs.expr->loc, expr,
				"Match case is not assignable to result type");
			return;
		}
		_case->value = lower_implicit_cast(ctx,
			expr->result, _case->value);
		_case = _case->next;
		acase = acase->next;
	}

		struct type_tagged_union *tu = result_type.next;
		while (tu) {
			struct type_tagged_union *next = tu->next;
			free(tu);
			tu = next;
		}
	while (result) {
		struct type_tagged_union *next = result->next;
		free(result);
		result = next;
	}
}

diff --git a/src/gen.c b/src/gen.c
index 9a16da92..391cd461 100644
--- a/src/gen.c
+++ b/src/gen.c
@@ -1781,6 +1781,12 @@ gen_expr_compound_with(struct gen_context *ctx,
	struct gen_value gvout = gv_void;
	if (!out) {
		gvout = mkgtemp(ctx, expr->result, ".%d");
		if (expr->result->storage != STORAGE_NEVER && expr->result->size != 0) {
			// XXX: hack, to make gvout appear initialized to QBE
			struct qbe_value qout = mkqval(ctx, &gvout);
			struct qbe_value zero = constl(0);
			pushi(ctx->current, &qout, Q_COPY, &zero, NULL);
		}
	}
	scope->out = out;
	scope->result = gvout;
@@ -2415,6 +2421,12 @@ gen_expr_if_with(struct gen_context *ctx,
	struct gen_value gvout = gv_void;
	if (!out) {
		gvout = mkgtemp(ctx, expr->result, ".%d");
		if (expr->result->storage != STORAGE_NEVER && expr->result->size != 0) {
			// XXX: hack, to make gvout appear initialized to QBE
			struct qbe_value qout = mkqval(ctx, &gvout);
			struct qbe_value zero = constl(0);
			pushi(ctx->current, &qout, Q_COPY, &zero, NULL);
		}
	}

	struct qbe_statement ltrue, lfalse, lend;
@@ -2780,6 +2792,12 @@ gen_expr_match_with(struct gen_context *ctx,
	struct gen_value gvout = gv_void;
	if (!out) {
		gvout = mkgtemp(ctx, expr->result, ".%d");
		if (expr->result->storage != STORAGE_NEVER && expr->result->size != 0) {
			// XXX: hack, to make gvout appear initialized to QBE
			struct qbe_value qout = mkqval(ctx, &gvout);
			struct qbe_value zero = constl(0);
			pushi(ctx->current, &qout, Q_COPY, &zero, NULL);
		}
	}
	struct qbe_statement lout;
	struct qbe_value bout = mklabel(ctx, &lout, ".%d");
diff --git a/tests/18-match.ha b/tests/18-match.ha
index 98d5f818..42ba450d 100644
--- a/tests/18-match.ha
+++ b/tests/18-match.ha
@@ -354,6 +354,12 @@ fn label() void = {
	};
};

// make sure this compiles and also passes through QBE successfully
fn f() int = match (0: (int | void)) {
case int => abort();
case => abort();
};

export fn main() void = {
	tagged_ptr();
	tagged();
diff --git a/tests/20-if.ha b/tests/20-if.ha
index 4676fc23..56d0257e 100644
--- a/tests/20-if.ha
+++ b/tests/20-if.ha
@@ -56,8 +56,8 @@ fn or() void = {
};

fn tagged() void = {
	assert((if (true) 1u8 else 0i8) as u8 == 1);
	assert((if (false) 1u8 else 0i8) as i8 == 0);
	assert((if (true) 1u8 else 0i8): (i8 | u8) as u8 == 1);
	assert((if (false) 1u8 else 0i8): (i8 | u8) as i8 == 0);
};

type abool = bool;
@@ -69,6 +69,9 @@ fn alias() void = {
	abort("unreachable");
};

// ensure this compiles and also passes QBE
fn f() int = if (true) abort() else abort();

fn _never() never = {
	if (true) {
		rt::exit(0);
diff --git a/tests/26-regression.ha b/tests/26-regression.ha
index ad7a178c..dba02e8f 100644
--- a/tests/26-regression.ha
+++ b/tests/26-regression.ha
@@ -96,6 +96,11 @@ fn control_never2() (int | void) = {
	};
};

// ensure this compiles and also passes through QBE
fn f() int = {
	abort();
};

export fn main() void = {
	let t = thing {
		offs = 0,
@@ -225,7 +230,7 @@ export fn main() void = {
	assert((if (false) 0) is void);
	let v = if (false) 0;
	assert(v is void);
	assert((if (true) 0) is int);
	assert((if (true) 0): (int | void) is int);
	v = if (true) 0;
	assert(v is int);

-- 
2.47.1
Lorenz (xha) <me@xha.li>
Details
Message ID
<Z4qCrOFGPfx7UNkn@xha.li>
In-Reply-To
<20250117030718.3160-1-bgs@turminal.net> (view parent)
Sender timestamp
1737134268
DKIM signature
pass
Download raw message
On Fri, Jan 17, 2025 at 04:04:12AM +0100, Bor Grošelj Simić wrote:
> This is essentially the same change as 56f9fa50, but for match, if and
> compound expressions instead of switch. The reasoning behind this change
> is provided in the accompanying spec commit.
> 
> This commit makes harec comply with the new spec and ensures the change
> does not introduce problems in gen.
> 
> Breaking-Change: language
> Signed-off-by: Bor Grošelj Simić <bgs@turminal.net>
> ---
> This implements the spec change at
> https://lists.sr.ht/~sircmpwn/hare-dev/patches/56767
> 
>  src/check.c            | 121 +++++++++++++++++++++++++----------------
>  src/gen.c              |  18 ++++++
>  tests/18-match.ha      |   6 ++
>  tests/20-if.ha         |   7 ++-
>  tests/26-regression.ha |   7 ++-
>  5 files changed, 109 insertions(+), 50 deletions(-)
> 
> diff --git a/src/check.c b/src/check.c
> index dd91dd88..8d2dbfe5 100644
> --- a/src/check.c
> +++ b/src/check.c
> @@ -1759,10 +1759,31 @@ check_expr_compound(struct context *ctx,
>  		check_expression(ctx, yexpr, lexpr, NULL);
>  		list->next->expr = lexpr;
>  	}
> -	expr->result = type_store_reduce_result(ctx, aexpr->loc,
> -			scope->results);
> +	if (hint) {
> +		expr->result = hint;
> +	} else {
> +		expr->result = type_store_reduce_result(
> +			ctx, aexpr->loc, scope->results);
> +		if (expr->result == NULL) {
> +			error(ctx, aexpr->loc, expr,
> +				"Invalid result type (dangling or ambiguous null)");
> +			return;
> +		}
> +	}
>  
>  	for (struct yield *yield = scope->yields; yield;) {
> +		if (hint && !type_is_assignable(ctx, hint,
> +				(*yield->expression)->result)) {
> +			char *yieldtypename = gen_typename((*yield->expression)->result);
> +			char *hinttypename = gen_typename(hint);
> +			error(ctx, expr->loc, expr,
> +				"Branch type (%s) is not assignable to hint (%s)",
> +				yieldtypename, hinttypename);
> +			free(yieldtypename);
> +			free(hinttypename);
> +			return;
> +
> +		}
>  		*yield->expression = lower_implicit_cast(ctx, expr->result,
>  			*yield->expression);
>  
> @@ -2342,14 +2363,29 @@ check_expr_if(struct context *ctx,
>  	} else {
>  		false_branch->type = EXPR_LITERAL;
>  		false_branch->result = &builtin_type_void;
> +		false_branch->loc = expr->loc;
>  	}
> -	const struct type *fresult = false_branch->result;
> -	if (hint && type_is_assignable(ctx, hint, true_branch->result)
> -			&& type_is_assignable(ctx, hint, fresult)) {
> +	if (hint) {
> +		struct expression *branch = NULL;
> +		if (!type_is_assignable(ctx, hint, true_branch->result)) {
> +			branch = true_branch;
> +		} else if (!type_is_assignable(ctx, hint, false_branch->result)) {
> +			branch = false_branch;
> +		}
> +		if (branch) {
> +			char *branchtypename = gen_typename(branch->result);
> +			char *hinttypename = gen_typename(hint);
> +			error(ctx, branch->loc, expr,
> +				"Branch type (%s) is not assignable to hint (%s)",
> +				branchtypename, hinttypename);
> +			free(branchtypename);
> +			free(hinttypename);
> +			return;
> +		}
>  		expr->result = hint;
>  	} else {
>  		struct type_tagged_union _tags = {
> -			.type = fresult,
> +			.type = false_branch->result,
>  			.next = NULL,
>  		}, tags = {
>  			.type = true_branch->result,
> @@ -2466,10 +2502,7 @@ check_expr_match(struct context *ctx,
>  		return;
>  	}
>  
> -	struct type_tagged_union result_type = {0};
> -	struct type_tagged_union *tagged = &result_type,
> -		**next_tag = &tagged->next;
> -
> +	struct type_tagged_union *tagged = NULL, *result = NULL;
>  	struct match_case **next = &expr->match.cases, *_case = NULL;
>  	for (struct ast_match_case *acase = aexpr->match.cases;
>  			acase; acase = acase->next) {
> @@ -2535,50 +2568,44 @@ check_expr_match(struct context *ctx,
>  			scope_pop(&ctx->scope);
>  		}
>  
> -		if (expr->result == NULL) {
> -			expr->result = _case->value->result;
> -			tagged->type = expr->result;
> -		} else if (expr->result != _case->value->result) {
> -			tagged = *next_tag =
> -				xcalloc(1, sizeof(struct type_tagged_union));
> -			next_tag = &tagged->next;
> -			tagged->type = _case->value->result;
> +		if (tagged == NULL) {
> +			result = tagged = xcalloc(1, sizeof *tagged);
> +		} else if (tagged->type && tagged->type != _case->value->result) {
> +			tagged = tagged->next = xcalloc(1, sizeof *tagged);
>  		}
> +		tagged->type = _case->value->result;
>  	}
>  
> -	if (result_type.next) {
> -		if (hint) {
> -			expr->result = hint;
> -		} else {
> -			expr->result = type_store_reduce_result(
> -				ctx, aexpr->loc, &result_type);
> -			if (expr->result == NULL) {
> -				error(ctx, aexpr->loc, expr,
> -					"Invalid result type (dangling or ambiguous null)");
> -				return;
> -			}
> +	if (hint) {
> +		expr->result = hint;
> +	} else {
> +		expr->result = type_store_reduce_result(
> +			ctx, aexpr->loc, result);
> +		if (expr->result == NULL) {
> +			error(ctx, aexpr->loc, expr,
> +				"Invalid result type (dangling or ambiguous null)");
> +			return;
>  		}
> +	}
>  
> -		struct match_case *_case = expr->match.cases;
> -		struct ast_match_case *acase = aexpr->match.cases;
> -		while (_case) {
> -			if (hint && !type_is_assignable(ctx, hint, _case->value->result)) {
> -				error(ctx, acase->exprs.expr->loc, expr,
> -					"Match case is not assignable to result type");
> -				return;
> -			}
> -			_case->value = lower_implicit_cast(ctx, 
> -				expr->result, _case->value);
> -			_case = _case->next;
> -			acase = acase->next;
> +	_case = expr->match.cases;
> +	struct ast_match_case *acase = aexpr->match.cases;
> +	while (_case) {
> +		if (hint && !type_is_assignable(ctx, hint, _case->value->result)) {
> +			error(ctx, acase->exprs.expr->loc, expr,
> +				"Match case is not assignable to result type");
> +			return;
>  		}
> +		_case->value = lower_implicit_cast(ctx,
> +			expr->result, _case->value);
> +		_case = _case->next;
> +		acase = acase->next;
> +	}
>  
> -		struct type_tagged_union *tu = result_type.next;
> -		while (tu) {
> -			struct type_tagged_union *next = tu->next;
> -			free(tu);
> -			tu = next;
> -		}
> +	while (result) {
> +		struct type_tagged_union *next = result->next;
> +		free(result);
> +		result = next;
>  	}
>  }
>  
> diff --git a/src/gen.c b/src/gen.c
> index 9a16da92..391cd461 100644
> --- a/src/gen.c
> +++ b/src/gen.c
> @@ -1781,6 +1781,12 @@ gen_expr_compound_with(struct gen_context *ctx,
>  	struct gen_value gvout = gv_void;
>  	if (!out) {
>  		gvout = mkgtemp(ctx, expr->result, ".%d");
> +		if (expr->result->storage != STORAGE_NEVER && expr->result->size != 0) {
> +			// XXX: hack, to make gvout appear initialized to QBE
> +			struct qbe_value qout = mkqval(ctx, &gvout);
> +			struct qbe_value zero = constl(0);
> +			pushi(ctx->current, &qout, Q_COPY, &zero, NULL);
> +		}
>  	}
>  	scope->out = out;
>  	scope->result = gvout;
> @@ -2415,6 +2421,12 @@ gen_expr_if_with(struct gen_context *ctx,
>  	struct gen_value gvout = gv_void;
>  	if (!out) {
>  		gvout = mkgtemp(ctx, expr->result, ".%d");
> +		if (expr->result->storage != STORAGE_NEVER && expr->result->size != 0) {
> +			// XXX: hack, to make gvout appear initialized to QBE
> +			struct qbe_value qout = mkqval(ctx, &gvout);
> +			struct qbe_value zero = constl(0);
> +			pushi(ctx->current, &qout, Q_COPY, &zero, NULL);
> +		}
>  	}
>  
>  	struct qbe_statement ltrue, lfalse, lend;
> @@ -2780,6 +2792,12 @@ gen_expr_match_with(struct gen_context *ctx,
>  	struct gen_value gvout = gv_void;
>  	if (!out) {
>  		gvout = mkgtemp(ctx, expr->result, ".%d");
> +		if (expr->result->storage != STORAGE_NEVER && expr->result->size != 0) {
> +			// XXX: hack, to make gvout appear initialized to QBE
> +			struct qbe_value qout = mkqval(ctx, &gvout);
> +			struct qbe_value zero = constl(0);
> +			pushi(ctx->current, &qout, Q_COPY, &zero, NULL);
> +		}
>  	}
>  	struct qbe_statement lout;
>  	struct qbe_value bout = mklabel(ctx, &lout, ".%d");
> diff --git a/tests/18-match.ha b/tests/18-match.ha
> index 98d5f818..42ba450d 100644
> --- a/tests/18-match.ha
> +++ b/tests/18-match.ha
> @@ -354,6 +354,12 @@ fn label() void = {
>  	};
>  };
>  
> +// make sure this compiles and also passes through QBE successfully
> +fn f() int = match (0: (int | void)) {
> +case int => abort();
> +case => abort();
> +};
> +
>  export fn main() void = {
>  	tagged_ptr();
>  	tagged();
> diff --git a/tests/20-if.ha b/tests/20-if.ha
> index 4676fc23..56d0257e 100644
> --- a/tests/20-if.ha
> +++ b/tests/20-if.ha
> @@ -56,8 +56,8 @@ fn or() void = {
>  };
>  
>  fn tagged() void = {
> -	assert((if (true) 1u8 else 0i8) as u8 == 1);
> -	assert((if (false) 1u8 else 0i8) as i8 == 0);
> +	assert((if (true) 1u8 else 0i8): (i8 | u8) as u8 == 1);
> +	assert((if (false) 1u8 else 0i8): (i8 | u8) as i8 == 0);
>  };
>  
>  type abool = bool;
> @@ -69,6 +69,9 @@ fn alias() void = {
>  	abort("unreachable");
>  };
>  
> +// ensure this compiles and also passes QBE
> +fn f() int = if (true) abort() else abort();
> +
>  fn _never() never = {
>  	if (true) {
>  		rt::exit(0);
> diff --git a/tests/26-regression.ha b/tests/26-regression.ha
> index ad7a178c..dba02e8f 100644
> --- a/tests/26-regression.ha
> +++ b/tests/26-regression.ha
> @@ -96,6 +96,11 @@ fn control_never2() (int | void) = {
>  	};
>  };
>  
> +// ensure this compiles and also passes through QBE
> +fn f() int = {
> +	abort();
> +};
> +
>  export fn main() void = {
>  	let t = thing {
>  		offs = 0,
> @@ -225,7 +230,7 @@ export fn main() void = {
>  	assert((if (false) 0) is void);
>  	let v = if (false) 0;
>  	assert(v is void);
> -	assert((if (true) 0) is int);
> +	assert((if (true) 0): (int | void) is int);

i think what you are trying to test here is if the type hint thing
works, right? wouldn't it make more sense to use something like
u32, because the 0 literal is going to get lowered to int by default?

>  	v = if (true) 0;
>  	assert(v is int);
>  
> -- 
> 2.47.1
> 
Details
Message ID
<D74TLPNWRNZF.314KGAWYHR2VI@turminal.net>
In-Reply-To
<Z4qCrOFGPfx7UNkn@xha.li> (view parent)
Sender timestamp
1737168612
DKIM signature
pass
Download raw message
On Fri Jan 17, 2025 at 5:17 PM CET, Lorenz (xha) wrote:
> On Fri, Jan 17, 2025 at 04:04:12AM +0100, Bor Grošelj Simić wrote:
> > This is essentially the same change as 56f9fa50, but for match, if and
> > compound expressions instead of switch. The reasoning behind this change
> > is provided in the accompanying spec commit.
> > 
> > This commit makes harec comply with the new spec and ensures the change
> > does not introduce problems in gen.
> > 
> > Breaking-Change: language
> > Signed-off-by: Bor Grošelj Simić <bgs@turminal.net>
> > ---
> > This implements the spec change at
> > https://lists.sr.ht/~sircmpwn/hare-dev/patches/56767
> > 
> >  src/check.c            | 121 +++++++++++++++++++++++++----------------
> >  src/gen.c              |  18 ++++++
> >  tests/18-match.ha      |   6 ++
> >  tests/20-if.ha         |   7 ++-
> >  tests/26-regression.ha |   7 ++-
> >  5 files changed, 109 insertions(+), 50 deletions(-)
> > 
> > diff --git a/src/check.c b/src/check.c
> > index dd91dd88..8d2dbfe5 100644
> > --- a/src/check.c
> > +++ b/src/check.c
> > @@ -1759,10 +1759,31 @@ check_expr_compound(struct context *ctx,
> >  		check_expression(ctx, yexpr, lexpr, NULL);
> >  		list->next->expr = lexpr;
> >  	}
> > -	expr->result = type_store_reduce_result(ctx, aexpr->loc,
> > -			scope->results);
> > +	if (hint) {
> > +		expr->result = hint;
> > +	} else {
> > +		expr->result = type_store_reduce_result(
> > +			ctx, aexpr->loc, scope->results);
> > +		if (expr->result == NULL) {
> > +			error(ctx, aexpr->loc, expr,
> > +				"Invalid result type (dangling or ambiguous null)");
> > +			return;
> > +		}
> > +	}
> >  
> >  	for (struct yield *yield = scope->yields; yield;) {
> > +		if (hint && !type_is_assignable(ctx, hint,
> > +				(*yield->expression)->result)) {
> > +			char *yieldtypename = gen_typename((*yield->expression)->result);
> > +			char *hinttypename = gen_typename(hint);
> > +			error(ctx, expr->loc, expr,
> > +				"Branch type (%s) is not assignable to hint (%s)",
> > +				yieldtypename, hinttypename);
> > +			free(yieldtypename);
> > +			free(hinttypename);
> > +			return;
> > +
> > +		}
> >  		*yield->expression = lower_implicit_cast(ctx, expr->result,
> >  			*yield->expression);
> >  
> > @@ -2342,14 +2363,29 @@ check_expr_if(struct context *ctx,
> >  	} else {
> >  		false_branch->type = EXPR_LITERAL;
> >  		false_branch->result = &builtin_type_void;
> > +		false_branch->loc = expr->loc;
> >  	}
> > -	const struct type *fresult = false_branch->result;
> > -	if (hint && type_is_assignable(ctx, hint, true_branch->result)
> > -			&& type_is_assignable(ctx, hint, fresult)) {
> > +	if (hint) {
> > +		struct expression *branch = NULL;
> > +		if (!type_is_assignable(ctx, hint, true_branch->result)) {
> > +			branch = true_branch;
> > +		} else if (!type_is_assignable(ctx, hint, false_branch->result)) {
> > +			branch = false_branch;
> > +		}
> > +		if (branch) {
> > +			char *branchtypename = gen_typename(branch->result);
> > +			char *hinttypename = gen_typename(hint);
> > +			error(ctx, branch->loc, expr,
> > +				"Branch type (%s) is not assignable to hint (%s)",
> > +				branchtypename, hinttypename);
> > +			free(branchtypename);
> > +			free(hinttypename);
> > +			return;
> > +		}
> >  		expr->result = hint;
> >  	} else {
> >  		struct type_tagged_union _tags = {
> > -			.type = fresult,
> > +			.type = false_branch->result,
> >  			.next = NULL,
> >  		}, tags = {
> >  			.type = true_branch->result,
> > @@ -2466,10 +2502,7 @@ check_expr_match(struct context *ctx,
> >  		return;
> >  	}
> >  
> > -	struct type_tagged_union result_type = {0};
> > -	struct type_tagged_union *tagged = &result_type,
> > -		**next_tag = &tagged->next;
> > -
> > +	struct type_tagged_union *tagged = NULL, *result = NULL;
> >  	struct match_case **next = &expr->match.cases, *_case = NULL;
> >  	for (struct ast_match_case *acase = aexpr->match.cases;
> >  			acase; acase = acase->next) {
> > @@ -2535,50 +2568,44 @@ check_expr_match(struct context *ctx,
> >  			scope_pop(&ctx->scope);
> >  		}
> >  
> > -		if (expr->result == NULL) {
> > -			expr->result = _case->value->result;
> > -			tagged->type = expr->result;
> > -		} else if (expr->result != _case->value->result) {
> > -			tagged = *next_tag =
> > -				xcalloc(1, sizeof(struct type_tagged_union));
> > -			next_tag = &tagged->next;
> > -			tagged->type = _case->value->result;
> > +		if (tagged == NULL) {
> > +			result = tagged = xcalloc(1, sizeof *tagged);
> > +		} else if (tagged->type && tagged->type != _case->value->result) {
> > +			tagged = tagged->next = xcalloc(1, sizeof *tagged);
> >  		}
> > +		tagged->type = _case->value->result;
> >  	}
> >  
> > -	if (result_type.next) {
> > -		if (hint) {
> > -			expr->result = hint;
> > -		} else {
> > -			expr->result = type_store_reduce_result(
> > -				ctx, aexpr->loc, &result_type);
> > -			if (expr->result == NULL) {
> > -				error(ctx, aexpr->loc, expr,
> > -					"Invalid result type (dangling or ambiguous null)");
> > -				return;
> > -			}
> > +	if (hint) {
> > +		expr->result = hint;
> > +	} else {
> > +		expr->result = type_store_reduce_result(
> > +			ctx, aexpr->loc, result);
> > +		if (expr->result == NULL) {
> > +			error(ctx, aexpr->loc, expr,
> > +				"Invalid result type (dangling or ambiguous null)");
> > +			return;
> >  		}
> > +	}
> >  
> > -		struct match_case *_case = expr->match.cases;
> > -		struct ast_match_case *acase = aexpr->match.cases;
> > -		while (_case) {
> > -			if (hint && !type_is_assignable(ctx, hint, _case->value->result)) {
> > -				error(ctx, acase->exprs.expr->loc, expr,
> > -					"Match case is not assignable to result type");
> > -				return;
> > -			}
> > -			_case->value = lower_implicit_cast(ctx, 
> > -				expr->result, _case->value);
> > -			_case = _case->next;
> > -			acase = acase->next;
> > +	_case = expr->match.cases;
> > +	struct ast_match_case *acase = aexpr->match.cases;
> > +	while (_case) {
> > +		if (hint && !type_is_assignable(ctx, hint, _case->value->result)) {
> > +			error(ctx, acase->exprs.expr->loc, expr,
> > +				"Match case is not assignable to result type");
> > +			return;
> >  		}
> > +		_case->value = lower_implicit_cast(ctx,
> > +			expr->result, _case->value);
> > +		_case = _case->next;
> > +		acase = acase->next;
> > +	}
> >  
> > -		struct type_tagged_union *tu = result_type.next;
> > -		while (tu) {
> > -			struct type_tagged_union *next = tu->next;
> > -			free(tu);
> > -			tu = next;
> > -		}
> > +	while (result) {
> > +		struct type_tagged_union *next = result->next;
> > +		free(result);
> > +		result = next;
> >  	}
> >  }
> >  
> > diff --git a/src/gen.c b/src/gen.c
> > index 9a16da92..391cd461 100644
> > --- a/src/gen.c
> > +++ b/src/gen.c
> > @@ -1781,6 +1781,12 @@ gen_expr_compound_with(struct gen_context *ctx,
> >  	struct gen_value gvout = gv_void;
> >  	if (!out) {
> >  		gvout = mkgtemp(ctx, expr->result, ".%d");
> > +		if (expr->result->storage != STORAGE_NEVER && expr->result->size != 0) {
> > +			// XXX: hack, to make gvout appear initialized to QBE
> > +			struct qbe_value qout = mkqval(ctx, &gvout);
> > +			struct qbe_value zero = constl(0);
> > +			pushi(ctx->current, &qout, Q_COPY, &zero, NULL);
> > +		}
> >  	}
> >  	scope->out = out;
> >  	scope->result = gvout;
> > @@ -2415,6 +2421,12 @@ gen_expr_if_with(struct gen_context *ctx,
> >  	struct gen_value gvout = gv_void;
> >  	if (!out) {
> >  		gvout = mkgtemp(ctx, expr->result, ".%d");
> > +		if (expr->result->storage != STORAGE_NEVER && expr->result->size != 0) {
> > +			// XXX: hack, to make gvout appear initialized to QBE
> > +			struct qbe_value qout = mkqval(ctx, &gvout);
> > +			struct qbe_value zero = constl(0);
> > +			pushi(ctx->current, &qout, Q_COPY, &zero, NULL);
> > +		}
> >  	}
> >  
> >  	struct qbe_statement ltrue, lfalse, lend;
> > @@ -2780,6 +2792,12 @@ gen_expr_match_with(struct gen_context *ctx,
> >  	struct gen_value gvout = gv_void;
> >  	if (!out) {
> >  		gvout = mkgtemp(ctx, expr->result, ".%d");
> > +		if (expr->result->storage != STORAGE_NEVER && expr->result->size != 0) {
> > +			// XXX: hack, to make gvout appear initialized to QBE
> > +			struct qbe_value qout = mkqval(ctx, &gvout);
> > +			struct qbe_value zero = constl(0);
> > +			pushi(ctx->current, &qout, Q_COPY, &zero, NULL);
> > +		}
> >  	}
> >  	struct qbe_statement lout;
> >  	struct qbe_value bout = mklabel(ctx, &lout, ".%d");
> > diff --git a/tests/18-match.ha b/tests/18-match.ha
> > index 98d5f818..42ba450d 100644
> > --- a/tests/18-match.ha
> > +++ b/tests/18-match.ha
> > @@ -354,6 +354,12 @@ fn label() void = {
> >  	};
> >  };
> >  
> > +// make sure this compiles and also passes through QBE successfully
> > +fn f() int = match (0: (int | void)) {
> > +case int => abort();
> > +case => abort();
> > +};
> > +
> >  export fn main() void = {
> >  	tagged_ptr();
> >  	tagged();
> > diff --git a/tests/20-if.ha b/tests/20-if.ha
> > index 4676fc23..56d0257e 100644
> > --- a/tests/20-if.ha
> > +++ b/tests/20-if.ha
> > @@ -56,8 +56,8 @@ fn or() void = {
> >  };
> >  
> >  fn tagged() void = {
> > -	assert((if (true) 1u8 else 0i8) as u8 == 1);
> > -	assert((if (false) 1u8 else 0i8) as i8 == 0);
> > +	assert((if (true) 1u8 else 0i8): (i8 | u8) as u8 == 1);
> > +	assert((if (false) 1u8 else 0i8): (i8 | u8) as i8 == 0);
> >  };
> >  
> >  type abool = bool;
> > @@ -69,6 +69,9 @@ fn alias() void = {
> >  	abort("unreachable");
> >  };
> >  
> > +// ensure this compiles and also passes QBE
> > +fn f() int = if (true) abort() else abort();
> > +
> >  fn _never() never = {
> >  	if (true) {
> >  		rt::exit(0);
> > diff --git a/tests/26-regression.ha b/tests/26-regression.ha
> > index ad7a178c..dba02e8f 100644
> > --- a/tests/26-regression.ha
> > +++ b/tests/26-regression.ha
> > @@ -96,6 +96,11 @@ fn control_never2() (int | void) = {
> >  	};
> >  };
> >  
> > +// ensure this compiles and also passes through QBE
> > +fn f() int = {
> > +	abort();
> > +};
> > +
> >  export fn main() void = {
> >  	let t = thing {
> >  		offs = 0,
> > @@ -225,7 +230,7 @@ export fn main() void = {
> >  	assert((if (false) 0) is void);
> >  	let v = if (false) 0;
> >  	assert(v is void);
> > -	assert((if (true) 0) is int);
> > +	assert((if (true) 0): (int | void) is int);
>
> i think what you are trying to test here is if the type hint thing
> works, right? wouldn't it make more sense to use something like
> u32, because the 0 literal is going to get lowered to int by default?

Whoops, that's from an unrelated change, shouldn't have been part of
this patch. Will send v2 without it.
Details
Message ID
<D751Q6Z8MMNS.332HAG75KIAO0@turminal.net>
In-Reply-To
<Z4qCrOFGPfx7UNkn@xha.li> (view parent)
Sender timestamp
1737191532
DKIM signature
pass
Download raw message
On Fri Jan 17, 2025 at 5:17 PM CET, Lorenz (xha) wrote:
> On Fri, Jan 17, 2025 at 04:04:12AM +0100, Bor Grošelj Simić wrote:
> > This is essentially the same change as 56f9fa50, but for match, if and
> > compound expressions instead of switch. The reasoning behind this change
> > is provided in the accompanying spec commit.
> > 
> > This commit makes harec comply with the new spec and ensures the change
> > does not introduce problems in gen.
> > 
> > Breaking-Change: language
> > Signed-off-by: Bor Grošelj Simić <bgs@turminal.net>
> > ---
> > This implements the spec change at
> > https://lists.sr.ht/~sircmpwn/hare-dev/patches/56767
> > 
> >  src/check.c            | 121 +++++++++++++++++++++++++----------------
> >  src/gen.c              |  18 ++++++
> >  tests/18-match.ha      |   6 ++
> >  tests/20-if.ha         |   7 ++-
> >  tests/26-regression.ha |   7 ++-
> >  5 files changed, 109 insertions(+), 50 deletions(-)
> > 
> > diff --git a/src/check.c b/src/check.c
> > index dd91dd88..8d2dbfe5 100644
> > --- a/src/check.c
> > +++ b/src/check.c
> > @@ -1759,10 +1759,31 @@ check_expr_compound(struct context *ctx,
> >  		check_expression(ctx, yexpr, lexpr, NULL);
> >  		list->next->expr = lexpr;
> >  	}
> > -	expr->result = type_store_reduce_result(ctx, aexpr->loc,
> > -			scope->results);
> > +	if (hint) {
> > +		expr->result = hint;
> > +	} else {
> > +		expr->result = type_store_reduce_result(
> > +			ctx, aexpr->loc, scope->results);
> > +		if (expr->result == NULL) {
> > +			error(ctx, aexpr->loc, expr,
> > +				"Invalid result type (dangling or ambiguous null)");
> > +			return;
> > +		}
> > +	}
> >  
> >  	for (struct yield *yield = scope->yields; yield;) {
> > +		if (hint && !type_is_assignable(ctx, hint,
> > +				(*yield->expression)->result)) {
> > +			char *yieldtypename = gen_typename((*yield->expression)->result);
> > +			char *hinttypename = gen_typename(hint);
> > +			error(ctx, expr->loc, expr,
> > +				"Branch type (%s) is not assignable to hint (%s)",
> > +				yieldtypename, hinttypename);
> > +			free(yieldtypename);
> > +			free(hinttypename);
> > +			return;
> > +
> > +		}
> >  		*yield->expression = lower_implicit_cast(ctx, expr->result,
> >  			*yield->expression);
> >  
> > @@ -2342,14 +2363,29 @@ check_expr_if(struct context *ctx,
> >  	} else {
> >  		false_branch->type = EXPR_LITERAL;
> >  		false_branch->result = &builtin_type_void;
> > +		false_branch->loc = expr->loc;
> >  	}
> > -	const struct type *fresult = false_branch->result;
> > -	if (hint && type_is_assignable(ctx, hint, true_branch->result)
> > -			&& type_is_assignable(ctx, hint, fresult)) {
> > +	if (hint) {
> > +		struct expression *branch = NULL;
> > +		if (!type_is_assignable(ctx, hint, true_branch->result)) {
> > +			branch = true_branch;
> > +		} else if (!type_is_assignable(ctx, hint, false_branch->result)) {
> > +			branch = false_branch;
> > +		}
> > +		if (branch) {
> > +			char *branchtypename = gen_typename(branch->result);
> > +			char *hinttypename = gen_typename(hint);
> > +			error(ctx, branch->loc, expr,
> > +				"Branch type (%s) is not assignable to hint (%s)",
> > +				branchtypename, hinttypename);
> > +			free(branchtypename);
> > +			free(hinttypename);
> > +			return;
> > +		}
> >  		expr->result = hint;
> >  	} else {
> >  		struct type_tagged_union _tags = {
> > -			.type = fresult,
> > +			.type = false_branch->result,
> >  			.next = NULL,
> >  		}, tags = {
> >  			.type = true_branch->result,
> > @@ -2466,10 +2502,7 @@ check_expr_match(struct context *ctx,
> >  		return;
> >  	}
> >  
> > -	struct type_tagged_union result_type = {0};
> > -	struct type_tagged_union *tagged = &result_type,
> > -		**next_tag = &tagged->next;
> > -
> > +	struct type_tagged_union *tagged = NULL, *result = NULL;
> >  	struct match_case **next = &expr->match.cases, *_case = NULL;
> >  	for (struct ast_match_case *acase = aexpr->match.cases;
> >  			acase; acase = acase->next) {
> > @@ -2535,50 +2568,44 @@ check_expr_match(struct context *ctx,
> >  			scope_pop(&ctx->scope);
> >  		}
> >  
> > -		if (expr->result == NULL) {
> > -			expr->result = _case->value->result;
> > -			tagged->type = expr->result;
> > -		} else if (expr->result != _case->value->result) {
> > -			tagged = *next_tag =
> > -				xcalloc(1, sizeof(struct type_tagged_union));
> > -			next_tag = &tagged->next;
> > -			tagged->type = _case->value->result;
> > +		if (tagged == NULL) {
> > +			result = tagged = xcalloc(1, sizeof *tagged);
> > +		} else if (tagged->type && tagged->type != _case->value->result) {
> > +			tagged = tagged->next = xcalloc(1, sizeof *tagged);
> >  		}
> > +		tagged->type = _case->value->result;
> >  	}
> >  
> > -	if (result_type.next) {
> > -		if (hint) {
> > -			expr->result = hint;
> > -		} else {
> > -			expr->result = type_store_reduce_result(
> > -				ctx, aexpr->loc, &result_type);
> > -			if (expr->result == NULL) {
> > -				error(ctx, aexpr->loc, expr,
> > -					"Invalid result type (dangling or ambiguous null)");
> > -				return;
> > -			}
> > +	if (hint) {
> > +		expr->result = hint;
> > +	} else {
> > +		expr->result = type_store_reduce_result(
> > +			ctx, aexpr->loc, result);
> > +		if (expr->result == NULL) {
> > +			error(ctx, aexpr->loc, expr,
> > +				"Invalid result type (dangling or ambiguous null)");
> > +			return;
> >  		}
> > +	}
> >  
> > -		struct match_case *_case = expr->match.cases;
> > -		struct ast_match_case *acase = aexpr->match.cases;
> > -		while (_case) {
> > -			if (hint && !type_is_assignable(ctx, hint, _case->value->result)) {
> > -				error(ctx, acase->exprs.expr->loc, expr,
> > -					"Match case is not assignable to result type");
> > -				return;
> > -			}
> > -			_case->value = lower_implicit_cast(ctx, 
> > -				expr->result, _case->value);
> > -			_case = _case->next;
> > -			acase = acase->next;
> > +	_case = expr->match.cases;
> > +	struct ast_match_case *acase = aexpr->match.cases;
> > +	while (_case) {
> > +		if (hint && !type_is_assignable(ctx, hint, _case->value->result)) {
> > +			error(ctx, acase->exprs.expr->loc, expr,
> > +				"Match case is not assignable to result type");
> > +			return;
> >  		}
> > +		_case->value = lower_implicit_cast(ctx,
> > +			expr->result, _case->value);
> > +		_case = _case->next;
> > +		acase = acase->next;
> > +	}
> >  
> > -		struct type_tagged_union *tu = result_type.next;
> > -		while (tu) {
> > -			struct type_tagged_union *next = tu->next;
> > -			free(tu);
> > -			tu = next;
> > -		}
> > +	while (result) {
> > +		struct type_tagged_union *next = result->next;
> > +		free(result);
> > +		result = next;
> >  	}
> >  }
> >  
> > diff --git a/src/gen.c b/src/gen.c
> > index 9a16da92..391cd461 100644
> > --- a/src/gen.c
> > +++ b/src/gen.c
> > @@ -1781,6 +1781,12 @@ gen_expr_compound_with(struct gen_context *ctx,
> >  	struct gen_value gvout = gv_void;
> >  	if (!out) {
> >  		gvout = mkgtemp(ctx, expr->result, ".%d");
> > +		if (expr->result->storage != STORAGE_NEVER && expr->result->size != 0) {
> > +			// XXX: hack, to make gvout appear initialized to QBE
> > +			struct qbe_value qout = mkqval(ctx, &gvout);
> > +			struct qbe_value zero = constl(0);
> > +			pushi(ctx->current, &qout, Q_COPY, &zero, NULL);
> > +		}
> >  	}
> >  	scope->out = out;
> >  	scope->result = gvout;
> > @@ -2415,6 +2421,12 @@ gen_expr_if_with(struct gen_context *ctx,
> >  	struct gen_value gvout = gv_void;
> >  	if (!out) {
> >  		gvout = mkgtemp(ctx, expr->result, ".%d");
> > +		if (expr->result->storage != STORAGE_NEVER && expr->result->size != 0) {
> > +			// XXX: hack, to make gvout appear initialized to QBE
> > +			struct qbe_value qout = mkqval(ctx, &gvout);
> > +			struct qbe_value zero = constl(0);
> > +			pushi(ctx->current, &qout, Q_COPY, &zero, NULL);
> > +		}
> >  	}
> >  
> >  	struct qbe_statement ltrue, lfalse, lend;
> > @@ -2780,6 +2792,12 @@ gen_expr_match_with(struct gen_context *ctx,
> >  	struct gen_value gvout = gv_void;
> >  	if (!out) {
> >  		gvout = mkgtemp(ctx, expr->result, ".%d");
> > +		if (expr->result->storage != STORAGE_NEVER && expr->result->size != 0) {
> > +			// XXX: hack, to make gvout appear initialized to QBE
> > +			struct qbe_value qout = mkqval(ctx, &gvout);
> > +			struct qbe_value zero = constl(0);
> > +			pushi(ctx->current, &qout, Q_COPY, &zero, NULL);
> > +		}
> >  	}
> >  	struct qbe_statement lout;
> >  	struct qbe_value bout = mklabel(ctx, &lout, ".%d");
> > diff --git a/tests/18-match.ha b/tests/18-match.ha
> > index 98d5f818..42ba450d 100644
> > --- a/tests/18-match.ha
> > +++ b/tests/18-match.ha
> > @@ -354,6 +354,12 @@ fn label() void = {
> >  	};
> >  };
> >  
> > +// make sure this compiles and also passes through QBE successfully
> > +fn f() int = match (0: (int | void)) {
> > +case int => abort();
> > +case => abort();
> > +};
> > +
> >  export fn main() void = {
> >  	tagged_ptr();
> >  	tagged();
> > diff --git a/tests/20-if.ha b/tests/20-if.ha
> > index 4676fc23..56d0257e 100644
> > --- a/tests/20-if.ha
> > +++ b/tests/20-if.ha
> > @@ -56,8 +56,8 @@ fn or() void = {
> >  };
> >  
> >  fn tagged() void = {
> > -	assert((if (true) 1u8 else 0i8) as u8 == 1);
> > -	assert((if (false) 1u8 else 0i8) as i8 == 0);
> > +	assert((if (true) 1u8 else 0i8): (i8 | u8) as u8 == 1);
> > +	assert((if (false) 1u8 else 0i8): (i8 | u8) as i8 == 0);
> >  };
> >  
> >  type abool = bool;
> > @@ -69,6 +69,9 @@ fn alias() void = {
> >  	abort("unreachable");
> >  };
> >  
> > +// ensure this compiles and also passes QBE
> > +fn f() int = if (true) abort() else abort();
> > +
> >  fn _never() never = {
> >  	if (true) {
> >  		rt::exit(0);
> > diff --git a/tests/26-regression.ha b/tests/26-regression.ha
> > index ad7a178c..dba02e8f 100644
> > --- a/tests/26-regression.ha
> > +++ b/tests/26-regression.ha
> > @@ -96,6 +96,11 @@ fn control_never2() (int | void) = {
> >  	};
> >  };
> >  
> > +// ensure this compiles and also passes through QBE
> > +fn f() int = {
> > +	abort();
> > +};
> > +
> >  export fn main() void = {
> >  	let t = thing {
> >  		offs = 0,
> > @@ -225,7 +230,7 @@ export fn main() void = {
> >  	assert((if (false) 0) is void);
> >  	let v = if (false) 0;
> >  	assert(v is void);
> > -	assert((if (true) 0) is int);
> > +	assert((if (true) 0): (int | void) is int);
>
> i think what you are trying to test here is if the type hint thing
> works, right? wouldn't it make more sense to use something like
> u32, because the 0 literal is going to get lowered to int by default?

Turns out this part of this patch on purpose, so to answer your question:
I think the int/u32 distintion does not matter here for what the
original test was trying to achieve. My modification is only here so
that the code compiles under the new slightly stricter rules, and the
"contents" of this test are intended to remain the same.
Reply to thread Export thread (mbox)