Re: [RFC PATCH 2/2] Specialize impl declarations on impl type in addition to decl type

[Ori Bernstein <ori@xxxxxxxxxxxxxx>]

Overall, the code looks pretty good.

There's only one minor issue I've been able to find so far. It seems like
this code prematurely rejects a trait specialized over a generic:

	use std

	trait foo @a =
		X	: int
	;;

	impl foo @a[:] =
		X = 123
	;;

	impl foo byte[:] =
		X = 234
	;;


	const main = {
		std.put("{}\n", impl(X, int[:]))
		//std.put("{}\n", impl(X, byte[:]))
	}

This is pretty minor, since it only seems to break the new use of ->param,
where there's no type to specialize over. I haven't dug in too deep to debug
yet, but it's clear that it's trying to decide which trait is associated
before doing the substitutions.

The way that traits were initially designed to work is that impls didn't
actually participate in type inference at all; The generics would get
specialized with appropriate traits, and only after inference was complete
would any checks be done.

I haven't dug in too deeply, but I think bestimpl in specialize.c needs to
know about the param in order to fix this.

On Sat,  1 Jul 2017 14:43:22 -0700, Michael Forney <mforney@xxxxxxxxxxx> wrote:

> This allows multiple specializations of a declarations with a concrete type,
> which can be selected with the new impl expression if it can't be deduced by its
> type alone.
> 
> For example
> 
> 	trait hasname @t =
> 		Name: byte[:]
> 	;;
> 	impl hasname void =
> 		Name = "somename"
> 	;;
> 	impl hasname bool =
> 		Name = "othername"
> 	;;
> 	const boolname = impl(Name, void)
> 
> To do this, pass the param type through to genericname and specializedcl. Since
> we now need the type parameter to look up trait decls, make sure n->expr.param
> gets the necessary treatment in typesub, specializenode, pickle, and unpickle.
> 
> We also need to tag the param types for export.
> ---
>  doc/lang.txt               |  4 +++-
>  mbld/deps.myr              |  2 +-
>  parse/export.c             |  2 ++
>  parse/infer.c              | 10 ++++++----
>  parse/parse.h              |  6 +++---
>  parse/specialize.c         | 16 +++++++++++-----
>  parse/use.c                |  5 +++++
>  test/implexpr-concrete.myr | 15 +++++++++++++++
>  test/tests                 |  1 +
>  9 files changed, 47 insertions(+), 14 deletions(-)
>  create mode 100644 test/implexpr-concrete.myr
> 
> diff --git a/doc/lang.txt b/doc/lang.txt
> index 3be5500f..d71bb70c 100644
> --- a/doc/lang.txt
> +++ b/doc/lang.txt
> @@ -1268,7 +1268,9 @@ TABLE OF CONTENTS:
>  
>              An impl expression chooses the implementation of the given trait
>              declaration for the given type. It is useful for refering to trait
> -            declarations in a generic context.
> +            declarations in a generic context. It also allows you to
> +            disambiguate a trait declaration whose type does not refer to the
> +            trait parameter.
>  
>          5.2.4. Cast Expressions:
>  
> diff --git a/mbld/deps.myr b/mbld/deps.myr
> index 1bdcae7f..506fffa3 100644
> --- a/mbld/deps.myr
> +++ b/mbld/deps.myr
> @@ -11,7 +11,7 @@ pkg bld =
>  	const myrdeps	: (b : build#, mt : myrtarg#, doclean : bool, addsrc : bool -> depgraph#)
>  ;;
>  
> -const Abiversion = 12
> +const Abiversion = 13
>  
>  var usepat	: regex.regex#
>  var cflagpat	: regex.regex#
> diff --git a/parse/export.c b/parse/export.c
> index 0c9be87d..4f67019f 100644
> --- a/parse/export.c
> +++ b/parse/export.c
> @@ -150,6 +150,8 @@ static void tagnode(Stab *st, Node *n, int ingeneric, int hidelocal)
>  	case Nexpr:
>  		tagnode(st, n->expr.idx, ingeneric, hidelocal);
>  		tagtype(st, n->expr.type, ingeneric, hidelocal);
> +		if (n->expr.param)
> +			tagtype(st, n->expr.param, ingeneric, hidelocal);
>  		for (i = 0; i < n->expr.nargs; i++)
>  			tagnode(st, n->expr.args[i], ingeneric, hidelocal);
>  		/* generics need to have the decls they refer to exported. */
> diff --git a/parse/infer.c b/parse/infer.c
> index 445b34b4..100ea83e 100644
> --- a/parse/infer.c
> +++ b/parse/infer.c
> @@ -1814,7 +1814,7 @@ static void specializeimpl(Inferstate *st, Node *n)
>  		unify(st, n, type(st, dcl), ty);
>  
>  		/* and put the specialization into the global stab */
> -		name = genericname(proto, ty);
> +		name = genericname(proto, n->impl.type, ty);
>  		sym = getdcl(file->file.globls, name);
>  		if (sym)
>  			fatal(n, "trait %s already specialized with %s on %s:%d",
> @@ -2423,6 +2423,8 @@ static void typesub(Inferstate *st, Node *n, int noerr)
>  		break;
>  	case Nexpr:
>  		settype(st, n, tyfix(st, n, type(st, n), 0));
> +		if (n->expr.param)
> +			n->expr.param = tyfix(st, n, n->expr.param, 0);
>  		typesub(st, n->expr.idx, noerr);
>  		if (exprop(n) == Ocast && exprop(n->expr.args[0]) == Olit &&
>  				n->expr.args[0]->expr.args[0]->lit.littype == Lint) {
> @@ -2493,7 +2495,7 @@ static void specialize(Inferstate *st, Node *f)
>  		pushstab(st->specializationscope[i]);
>  		n = st->specializations[i];
>  		if (n->type == Nexpr) {
> -			d = specializedcl(st->genericdecls[i], n->expr.type, &name);
> +			d = specializedcl(st->genericdecls[i], n->expr.param, n->expr.type, &name);
>  			n->expr.args[0] = name;
>  			n->expr.did = d->decl.did;
>  
> @@ -2506,11 +2508,11 @@ static void specialize(Inferstate *st, Node *f)
>  			ty = exprtype(n->iterstmt.seq);
>  
>  			it = itertype(st, n->iterstmt.seq, mktype(n->loc, Tybool));
> -			d = specializedcl(tr->proto[0], it, &name);
> +			d = specializedcl(tr->proto[0], ty, it, &name);
>  			htput(tr->proto[0]->decl.impls, ty, d);
>  
>  			it = itertype(st, n->iterstmt.seq, mktype(n->loc, Tyvoid));
> -			d = specializedcl(tr->proto[1], it, &name);
> +			d = specializedcl(tr->proto[1], ty, it, &name);
>  			htput(tr->proto[1]->decl.impls, ty, d);
>  		} else {
>  			die("unknown node for specialization\n");
> diff --git a/parse/parse.h b/parse/parse.h
> index 6faeba2b..b0d56289 100644
> --- a/parse/parse.h
> +++ b/parse/parse.h
> @@ -1,4 +1,4 @@
> -#define Abiversion 12
> +#define Abiversion 13
>  
>  typedef struct Srcloc Srcloc;
>  typedef struct Tysubst Tysubst;
> @@ -508,9 +508,9 @@ void substput(Tysubst *subst, Type *from, Type *to);
>  Type *substget(Tysubst *subst, Type *from);
>  void substpush(Tysubst *subst);
>  void substpop(Tysubst *subst);
> -Node *specializedcl(Node *n, Type *to, Node **name);
> +Node *specializedcl(Node *n, Type *param, Type *to, Node **name);
>  Type *tyspecialize(Type *t, Tysubst *tymap, Htab *delayed, Htab *tybase);
> -Node *genericname(Node *n, Type *t);
> +Node *genericname(Node *n, Type *param, Type *t);
>  void geninit(Node *file);
>  
>  /* usefiles */
> diff --git a/parse/specialize.c b/parse/specialize.c
> index e9805872..dbc2d07c 100644
> --- a/parse/specialize.c
> +++ b/parse/specialize.c
> @@ -241,7 +241,7 @@ static void fixup(Node *n)
>  			if (!d)
>  				die("Missing decl %s", namestr(n->expr.args[0]));
>  			if (d->decl.isgeneric)
> -				d = specializedcl(d, n->expr.type, &n->expr.args[0]);
> +				d = specializedcl(d, n->expr.param, n->expr.type, &n->expr.args[0]);
>  			n->expr.did = d->decl.did;
>  		}
>  		break;
> @@ -328,6 +328,8 @@ static Node *specializenode(Node *n, Tysubst *tsmap)
>  	case Nexpr:
>  		r->expr.op = n->expr.op;
>  		r->expr.type = tysubst(n->expr.type, tsmap);
> +		if (n->expr.param)
> +			r->expr.param = tysubst(n->expr.param, tsmap);
>  		r->expr.isconst = n->expr.isconst;
>  		r->expr.nargs = n->expr.nargs;
>  		r->expr.idx = specializenode(n->expr.idx, tsmap);
> @@ -435,7 +437,7 @@ static Node *specializenode(Node *n, Tysubst *tsmap)
>  	return r;
>  }
>  
> -Node *genericname(Node *n, Type *t)
> +Node *genericname(Node *n, Type *param, Type *t)
>  {
>  	char buf[1024];
>  	char *p;
> @@ -447,6 +449,10 @@ Node *genericname(Node *n, Type *t)
>  	p = buf;
>  	end = buf + sizeof buf;
>  	p += bprintf(p, end - p, "%s", n->decl.name->name.name);
> +	if (param) {
> +		p += bprintf(p, end - p, "$");
> +		p += tyidfmt(p, end - p, param);
> +	}
>  	p += bprintf(p, end - p, "$");
>  	p += tyidfmt(p, end - p, t);
>  	name = mkname(n->loc, buf);
> @@ -574,7 +580,7 @@ Node *bestimpl(Node *n, Type *to)
>   * duplicate of it with type 'to'. It also generates
>   * a name for this specialized node, and returns it in '*name'.
>   */
> -Node *specializedcl(Node *gnode, Type *to, Node **name)
> +Node *specializedcl(Node *gnode, Type *param, Type *to, Node **name)
>  {
>  	extern int stabstkoff;
>  	Tysubst *tsmap;
> @@ -584,7 +590,7 @@ Node *specializedcl(Node *gnode, Type *to, Node **name)
>  	assert(gnode->type == Ndecl);
>  	assert(gnode->decl.isgeneric);
>  
> -	n = genericname(gnode, to);
> +	n = genericname(gnode, param, to);
>  	*name = n;
>  	if (n->name.ns)
>  		st = getns(file, n->name.ns);
> @@ -598,7 +604,7 @@ Node *specializedcl(Node *gnode, Type *to, Node **name)
>  	if (gnode->decl.trait) {
>  		g = bestimpl(gnode, to);
>  		if (!g)
> -			fatal(gnode, "no trait implemented for %s:%s", namestr(gnode->decl.name), tystr(to));
> +			fatal(gnode, "type %s does not implement %s:%s", tystr(param), namestr(gnode->decl.name), tystr(to));
>  	} else {
>  		g = gnode;
>  	}
> diff --git a/parse/use.c b/parse/use.c
> index 1c02a6b7..013a176d 100644
> --- a/parse/use.c
> +++ b/parse/use.c
> @@ -464,6 +464,9 @@ static void pickle(FILE *fd, Node *n)
>  	case Nexpr:
>  		wrbyte(fd, n->expr.op);
>  		wrtype(fd, n->expr.type);
> +		wrbool(fd, n->expr.param != NULL);
> +		if (n->expr.param)
> +			wrtype(fd, n->expr.param);
>  		wrbool(fd, n->expr.isconst);
>  		pickle(fd, n->expr.idx);
>  		wrint(fd, n->expr.nargs);
> @@ -598,6 +601,8 @@ static Node *unpickle(FILE *fd)
>  	case Nexpr:
>  		n->expr.op = rdbyte(fd);
>  		rdtype(fd, &n->expr.type);
> +		if (rdbool(fd))
> +			rdtype(fd, &n->expr.param);
>  		n->expr.isconst = rdbool(fd);
>  		n->expr.idx = unpickle(fd);
>  		n->expr.nargs = rdint(fd);
> diff --git a/test/implexpr-concrete.myr b/test/implexpr-concrete.myr
> new file mode 100644
> index 00000000..9d5eeb18
> --- /dev/null
> +++ b/test/implexpr-concrete.myr
> @@ -0,0 +1,15 @@
> +use std
> +
> +trait name @a =
> +	Name: byte[:]
> +;;
> +impl name void =
> +	Name = "zig"
> +;;
> +impl name int =
> +	Name = "zag"
> +;;
> +
> +const main = {
> +	std.put("{}{}\n", impl(Name, void), impl(Name, int))
> +}
> diff --git a/test/tests b/test/tests
> index 937828be..b7ab8e1d 100644
> --- a/test/tests
> +++ b/test/tests
> @@ -165,3 +165,4 @@ B nestedgoto	E	0
>  B initializer	E	0
>  B fmtalign	E	0
>  B implexpr	P	12,z,hello
> +B implexpr-concrete	P	zigzag
> -- 
> 2.13.2
> 
> 


-- 
    Ori Bernstein