gracelang / minigrace

Self-hosting compiler for the Grace programming language
39 stars 22 forks source link

alias underspecification bug #264

Closed KimBruce closed 5 years ago

KimBruce commented 6 years ago

As far as I can tell, the AST for an alias clause does not hold enough information to pick up certain errors. Consider the following code:

type A = {
    nm (l:String, k: Number) → Object
    p(s:String) → String
}

type B = {
        nm (l:String, k: Number) → Number
}

class a (n: Number) → A {
    method nm(l:String, k: Number) → Object {47}
    method p(s:String) → String {"bye"}
}

class b → B{
    inherit a(2)
        alias oldnm(k: Number, l: String) = nm(l,k)
        exclude p(s:String)
    method nm(l:String,k: Number) → Number {
        oldnm("hello",3) + 2
    }
    print(nm("s",47))
}
b

This code runs without error, returning a value of 49.

However, it should fail. Notice that in defining oldnm, I have reversed the order of the parameters, which are of type Number and String. Then when I call oldnm 3 lines down, I call it with bad arguments. I.e., the first argument should be a number and the second a string, yet it works fine.

There are a number of ways that might have been caught as illegal. The simplest is not to allow any changes to the order of parameters between the alias and the original (and similarly don't allow any changes to parts). I suspect this is what we intended, though the spec provides no help (almost all the example of alias and exclude have parameterless methods).

Alternatively, the actual call of oldnm could treated as illegal since the types don't match those specified in the alias statement. (However, currently reversing the arguments of the call results in a crash, so an "apparently" type-safe call is not.)

One issue seems to be that the representations of aliases in the AST does not contain sufficient information. E.g. oldnm above is simply represented as oldnm(2), indicating that it has two parameters, but not providing info on types or how they connect with the parameters of nm(2).

Exclude is perhaps less problematic, though you would likely want to know if the programmer wrote "exclude p(n:Number)" when p actually took a String argument. Again there is not sufficient info in the AST to pick this up.

apblack commented 6 years ago

I agree that the behaviour of aliases is under-specified. I disagree that the above program is illegal. Like method names in interface literals, method names in alias clauses may contain names for the parameters and type declarations, but those names are for documentation only, and have no effect on the semantics of he program.

The situation is similar to writing an overriding method declaration: the name and type of the parameter in the declaration being overridden, and in the overriding declaration, may be different. Indeed, the programmer writing an alias clause, or an overriding declaration, will not in general even know the name and type used in the original declaration, since this information may be in another module to which they do not have the source code.

Here is some proposed new text for the Grace Specification. It adds a paragraph just before the examples, changes some of the examples, and adds a counter-example. (Quoting the text makes the line-breaks hard; they will become soft again when the text is included in the spec.)

Reuse

Grace supports reuse in two ways: through inherit statements and through use statements. Object constructors (including classes and modules) can contain one inherit statement, while traits cannot contain an inherit statement; object constructors, classes, modules and traits can all contain one or more use statements. As a special case, an object with no inherit statements is treated as though it inherited from graceObject (although this implicit inheritance does not disqualify the object from being a trait).

Both inherit and use introduce the attributes of a reused object — called the parent — into the current object (the object under construction). There are two differences between inherit and use clauses:

  1. the object reused by a use clause must be a trait object; and
  2. inherit clauses include methods in the parent that originated in graceObject, while use clauses do not.

The parent in an inherit parent and use parent clause must be a Manifest Expression that returns a Fresh Object; usually this will be a request on a class or trait. The parent cannot depend on self, implicitly or explicitly, because self does not exist until after the reuse statement containing parent has been evaluated.

If it is necessary for the current object to access an overridden attribute of a parent, the overridden attribute can be given an additional name by attaching an alias clause to the inherit or use statement: alias new(_) = old(_) creates a new confidential alias new(_) for the attribute old(_). Attributes of the parent that are not wanted can be excluded using an exclude clause. It is an object composition error to alias or exclude attributes that are not present in the object being inherited, or to alias an attribute to its own name.

The set of attributes that is introduced by the reuse statement is determined as follows.

  • The set contains all the attributes of the parent, except for those that appear in an exclude clause (and, in the case of a use statement, except for the attributes obtained from graceObject).
  • The set contains all of the new attribute names introduced by the alias clauses. Each of these names is bound to the attribute to which the old method name is bound in the parent.

The order of the alias and exclude clauses is irrelevant.

Attributes introduced by an alias clause are treated as being introduced by the object under construction, and thus do not conflict with (and may therefore override) attributes obtained by reuse. They do conflict with attributes declared in the object under construction.

The method names in alias and exclude clauses have the same syntax as method names in method declarations and interface literals; this means that they can contain both parameter names and type annotations. Such names and annotations may be useful as documentation, but do not affect the meaning of the program.

Examples

trait t1 {
    method x(size:Number) { ... }
    method y(name:String) { ... }
}

class c1 {
    use t1 alias w(_) = y(_) exclude x(_)
    method v { ... }
}

Objects generated by c1 have attributes v, w(_) and y(_), but not x(_)

trait t1 {
    method x(size:Number) { ... }
    method y(name:String) { ... }
}

class c1 {
    use t1 alias w(name) = y(name) exclude x(_)
    method w(kind) { ... }
}

This is a trait composition error, because c1 gets w(_) from two places: an alias clause, and a method definition.

trait t1 {
    method x { ... }
    method y { ... }
}

class c1 {
    use t1 alias x = y
    method y is override { ... }
    method w { ... }
}

This is also a trait composition error, because x is defined twice: in t1, and in the alias clause. This can be corrected by excluding x from t1.

trait compare {
    method lessThanOrEqual (a, b) { a.name ≤ b.name }
}

trait moreCompare {
    use compare alias greaterThanOrEqual(x, y) = lessThanOrEqual(y, x)
}

In this example, the trait moreCompare has two methods, greaterThanOrEqual(_,_) and lessThanOrEqual(_,_), but these method are identical, and therefore the names are misleading. If the intension is to make greaterThanOrEqual(_,_) perform the inverse comparison, this should have been written as

trait moreCompare {
    use compare 
    method greaterThanOrEqual(x, y) is confidential {
        lessThanOrEqual(y, x)
    }
}

Of course, if a dialect wants to make additional restrictions, it can do so. To this end, minigrace commit e8814d2d7 includes in the parse tree methodNodes for the names in the alias and exclude clauses, as requested. Of course, if the reused object comes from another module, the compiler will not have any information about the names of their parameters.

KimBruce commented 6 years ago

Thanks for doing this. A couple of remarks/suggestions/questions:

If it is necessary for the current object to access an overridden attribute of a parent, the overridden attribute can be given an additional name by attaching an alias clause to the inherit or use statement: alias new(_) = old(_) creates a new confidential alias new(_) for the attribute old(_).

Can I suggest that you make new and old methods that take two parameters. This makes it clear that there should be one underscore for each parameter (and they should be separated by commas).

_Of course, if a dialect wants to make additional restrictions, it can do so. To this end, minigrace commit e8814d2d7 includes in the parse tree methodNodes for the names in the alias and exclude clauses, as requested. Of course, if the reused object comes from another module, the compiler will not have any information about the names of their parameters._

Names are less important than types, given the semantics described here.

A few questions about run-time semantics:

My intuition tells me that the alias would execute the same run-time checks as the original method (e.g., on parameters and return types). That should probably be mentioned (if true).

Should the alias clause be able to tighten up the parameter types and loosen the return types if specified in the alias? I'm guessing no, but we probably should say something about it.

Finally, is a confidential method defined by alias in a superclass visible in a subclass? E.g., if c2 inherits from your first c1, should c2 be able to see and use w(_)? I think the answer should be yes, but could be persuaded otherwise.

apblack commented 6 years ago

I think that it would be a good idea to make at least one of the examples use more than one argument. A three argument method, with different groupings, would be good, e.g., fix(_)with(_,_) = repair(_)using(_) and(_). More importantly, the text should use the term “canonical name”.

The whole spec would be clearer if the grammar were include in parts. We should try and do this before the workshop.

I think that the text is already pretty clear that an alias is just a new name for an existing attribute. This already answers all of your other questions. The only thing that contra-indicates that reading is that an alias somehow makes the attribute confidential, even if it were not otherwise confidential. I don’t like this, but that’s mainly because I don’t know how to implement it.

apblack commented 5 years ago

I'm closing this because the parse tree now contains the method nodes, which is the issue.

If there is a problem with the spec, it should be raised as a separate issue in graceLanguage. As far as I can see, the spec is clear, but that's probably because I wrote it :-)