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

[Ori Bernstein <ori@xxxxxxxxxxxxxx>]

Pushed it as is, since it doesn't introduce a regression -- whoever gets to it
first, though, should follow up and fix this.

On Sun, 2 Jul 2017 21:55:56 -0700, Ori Bernstein <ori@xxxxxxxxxxxxxx> wrote:

> 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
> 


-- 
    Ori Bernstein