eclipse-archived/ceylon

allow un-'shared' refinements

Opened this issue · 28 comments

Previously, we forced you to write shared actual FullyExplicitType name on refining member definitions when satisfying an interface or extending a class. I've always found this a bit too verbose and it's always made me uncomfortable.

I recently (#6347) dropped the requirement for fully-specifying the explicit type, but did not document that change in the spec, since there were objections from other folks here. People don't like muddying the idea that "all shared members need explicit types".

I've also very often thought that actual should imply shared, but didn't like breaking the orthogonality of the annotations.

Today I thought of a very interesting approach to reduce the verbosity of refinements: allow private refinements!

What I'm proposing (and already have a prototype implementation of) is the following:

If a member is annotated actual but not shared:

  • it may also be formal or default, and
  • it may have an inferred type, but
  • if it is formal or default, then its type must be exactly the same as every member it refines.

Since it is not shared, it is not visible outside the type in which it occurs. However, its existence
can be detected by subclasses, since it might implement a formal member of the supertype or
redeclare a non-formal member of the supertype as formal.

One thing I like about this is that shortcut refinement just becomes a special case of this, instead of adding a new shared member.

I'm not sure I understand what is being proposed. Is it that if B "privately" refines foo(), and I call b.foo() from somewhere, the code that runs is B.foo()'s definition, but if I command-click b.foo in the IDE, I'm sent to the refined declaration, e.g. A.foo()?

@jvasileff yeah, essentially.

But it also means that if B.foo() narrows the type of foo(), that narrowing is not visible outside of B.

OK, I just pushed an initial implementation you can try out. It's a rather huge diff, but there's nothing really very hard in there.

that narrowing is not visible outside of B.

That doesn't seem like a huge benefit to me. If formal or default, it would still add a requirement for C's implementation of foo(). I guess this would (if no backend issues) allow us to change our minds later about the type if the refinement is not formal or default without breaking compatibility, but the benefit seems small compared to the opacity of it all. Allowing refinements but not letting the outside world know about them seems confusing.

And, the effect (hiding the refinement) is seemingly not related to the usual intention, which is to reduce implementation verbosity. Rather, the hiding is just a negative side effect.

I think I'd prefer for shared to be implied, or for all of us to just be less curmudgeonly about your idea of using value for refinements.

@jvasileff To be clear, my main goal here is to reduce the verbosity of the code, without losing orthogonality of the language constructs. It's not that I think plain actual is especially meaningfully better than shared actual, rather, it's that I ask myself:

Why the hell do I have to annotate this thing shared when I'm not freaking introducing anything new into the schema of the type that's visible to clients!?

That shared annotation just isn't meaningful there (unless I'm narrowing the return type), but Ceylon forces me to write it in every time!

Example of a language module class rewritten using this feature:

class ArraySequence<out Element>(array)
        extends Object()
        satisfies [Element+] {

    Array<Element> array;

    assert (!array.empty);

    getFromFirst(Integer index)
            => array.getFromFirst(index);

    contains(Object element)
            => array.contains(element);

    size => array.size;

    iterator() => array.iterator();

    actual Element first {
        if (exists first = array.first) {
            return first;
        }
        else {
            assert (is Element null);
            return null;
        }
    }

    actual Element last {
        if (exists last = array.last) {
            return last;
        }
        else {
            assert (is Element null);
            return null;
        }
    }

    tuple() => nothing;

    each(void step(Element element)) => array.each(step);

    count(Boolean selecting(Element element))
            => array.count(selecting);

    every(Boolean selecting(Element element))
            => array.every(selecting);

    any(Boolean selecting(Element element))
            => array.any(selecting);

    find(Boolean selecting(Element&Object element))
            => array.find(selecting);

    findLast(Boolean selecting(Element&Object element))
            => array.findLast(selecting);

    actual Result|Element reduce<Result>(
            Result accumulating(Result|Element partial,
                    Element element)) {
        // cannot follow std pattern of narrowing null
        // https://github.com/ceylon/ceylon/issues/7021
        value result = array.reduce(accumulating);
        if (exists result) {
            return result;
        }
        else {
            assert (is Result|Element result);
            return result;
        }
    }

    actual function collect<Result>
            (Result collecting(Element element)) {
        assert (nonempty sequence
                = array.collect(collecting));
        return sequence;
    }

    actual function sort
            (Comparison comparing(Element x, Element y)) {
        assert (nonempty sequence
                = array.sort(comparing));
        return sequence;
    }

    actual function measure(Integer from, Integer length) {
        if (from > lastIndex ||
        length <= 0 ||
        from + length <= 0) {
            return [];
        }
        else {
            return ArraySequence(array[from : length]);
        }
    }

    actual function span(Integer from, Integer to) {
        if (from <= to) {
            return
            if (to < 0 || from > lastIndex)
            then []
            else ArraySequence(array[from..to]);
        }
        else {
            return
            if (from < 0 || to > lastIndex)
            then []
            else ArraySequence(array[from..to]);
        }
    }

    actual function spanFrom(Integer from) {
        if (from <= 0) {
            return this;
        }
        else if (from < size) {
            return ArraySequence(array[from...]);
        }
        else {
            return [];
        }
    }

    actual function spanTo(Integer to) {
        if (to >= lastIndex) {
            return this;
        }
        else if (to >= 0) {
            return ArraySequence(array[...to]);
        }
        else {
            return [];
        }
    }

}

That shared annotation just isn't meaningful there (unless I'm narrowing the return type), but Ceylon forces me to write it in every time!

I don't care too much about the shared annotation, but after a bit of thought, I believe that having refinements be shared is essential. Only in a very theoretical sense is it unimportant for a software developer to know what code is actually being run. There are too many reasons why a developer might care to list here.

If this affects shortcut refinement, it would make me stop using shortcut refinement. And I believe it would very much frustrate me to work on codebases that used concealed (haha) refinements, and strongly discourage me from troubleshooting or ad-hoc reviewing them.

I believe that having refinements be shared is essential.

Why? I don't think I follow.

I think I'd prefer for shared to be implied, or for all of us to just be less curmudgeonly about your idea of using value for refinements.

Note that in terms of what the programmer actually writes, there's almost no difference at all between my proposal and yours; the only difference is that, in my system, if you want to narrow the type, you have to annotate the thing shared.

If this affects shortcut refinement, it would make me stop using shortcut refinement.

In fact, after experimenting with that, I rolled it back. It turns out that there are useful situations where shortcut refinement narrows types, which is something disallowed for un-shared refinements. So my current prototype doesn't affect the existing semantics of shortcut refinement in any way.

I believe that having refinements be shared is essential.

Why? I don't think I follow.

It's:

Only in a very theoretical sense is it unimportant for a software developer to know what code is actually being run. There are too many reasons why a developer might care to list here.

but ok, I'll write some more stuff.

  1. If this is indeed true and the IDE won't offer a good guess at what code will run for b.foo(), that's a problem.
  2. The same for 'command-T' on a refined declaration, which should show all of the refinements
  3. While less critical, the same info should be available in ceylon-docs.
  4. If doc annotations are allowed, the missing ceylon-doc issue becomes a bigger problem, since the doc annotation may provide useful info about a functional refinement, even if the return type is the same
  5. AFICT, this adds complexity to the backends, as they will have to treat actual as shared

If all of these issues are addressed in some way, then aren't these "non" shared declarations in effect shared anyway? We'd just be complicating the code: instead of having actual imply shared in a single place, we'd have logic to treat actual as shared spread out in several places. The same with the rules of the language.

This makes things more complicated and more opaque with no benefit.

  1. If this is indeed true and the IDE won't offer a good guess at what code will run for b.foo(), that's a problem.

Well what the IDE does is up to the IDE, and I can make it do whatever we want.

  • Currently, navigation from a member reference already skips shortcut refinements (since they are considered assignments, not declarations, by the IDE), and
  • with this change it would also skip unshared refinements.

But I could change that.

  1. The same for 'command-T' on a refined declaration, which should show all of the refinements

I already made the two trivial changes to make that work in IntelliJ.

  1. While less critical, the same info should be available in ceylon-docs.

I'm not really seeing this, to be honest I wonder why we bother including shortcut refinements in ceylon doc output (they don't have documentation, nor do they declare any interesting type information).

But if it's really important to you, it would be surely a very trivial change to have actual unshared declarations included by default.

If all of these issues are addressed in some way, then aren't these "non" shared declarations in effect shared anyway?

Well, sure, that's what I tried to say above: from the point of view of the user, there difference is really minimal. On the other hand, from the point of view of cleanliness of the spec, it's actually a more elegant change.

We'd just be complicating the code: instead of having actual imply shared in a single place, we'd have logic to treat actual as shared spread out in several places.

I don't think it complicates the code. Sure, I should add a isSharedOrActual() method to Declaration instead of having d.isShared() || d.isActual() in several places in the backend. But overall it's a pretty mechanical replacement.

The same with the rules of the language.

I disagree. I view my proposal as the simpler change from the point of view of specifying the language rules. The only new rule is the one I mentioned above:

  • if it is [actual and either] formal or default, then its type must be exactly the same as every member it refines

Currently, navigation from a member reference already skips shortcut refinements

Yeah, that's confusing. Here's a case of that with different details, but same idea.

Well, sure, that's what I tried to say above: from the point of view of the user, there difference is really minimal. On the other hand, from the point of view of cleanliness of the spec, it's actually a more elegant change.

If we agree these should act as shared, why confuse the user by telling them that the declaration is not shared? But really, my main argument is about the implementation, not the documentation.

I don't think it complicates the code.

How can you argue that, vs. a very small change to flip the shared flag when the typechecker sees actual? What's wrong with using the shared flag, even if you want to document this as non-shared?

Yeah, that's confusing.

No problem, I can fix it.

But really, my main argument is about the implementation, not the documentation.
How can you argue that

What I think you're missing here is that the typechecker currently has (and needs) a special algorithm for type inference on refinements (i.e. take the intersection of the refined types). This is what everyone objected to in #6347.

Now, with this new proposal, that special algorithm could in principle be thrown away, and the regular type inference rules would be used.

That's a significant simplification it seems to me.

a special algorithm ... This is what everyone objected to

There's a whole lot of nuance with that argument I'd rather not get into. Like I said at the start, we should all just stop being so argumentative and accept your abbreviated syntax.

Now, with this new proposal, that special algorithm could in principle be thrown away, and the regular type inference rules would be used.

What about BC? This implicitly narrowed type information will wind up in byte code for public members, right?

What about BC? This implicitly narrowed type information will wind up in byte code for public members, right?

Hrm, interesting point. I had not thought of that. It's true, but on the other hand, Ceylon clients could not depend on that because they would not see the narrower type. Clients written in Java would, however.

I suspect the bytecode generated for Ceylon clients would indicate the narrowed return type, depending on javac's method resolution.

I suspect the bytecode generated for Ceylon clients would indicate the narrowed return type

Well, we could make absolutely sure this never happens by making the restriction I proposed above just a little more heavy-handed:

  • it may have either an inferred or explicit type, but
  • its type must be exactly the same as every member it refines

There's a whole lot of nuance with that argument I'd rather not get into. Like I said at the start, we should all just stop being so argumentative and accept your abbreviated syntax.

Well note: it's not that I'm trying to say I agree with the argument; I don't really, on the contrary I think what was implemented in #6347 is perfectly reasonable. The reason I brought it up was as a counterpoint to your assertion that what's proposed here is more complex. It's not; it requires one less type inference algorithm. (And more importantly, it keeps shared, actual and type inference as three completely orthogonal things, more orthogonal than they are today, in fact.)

@gavinking would it be worth restating the goals at this point?

Now that private covariant refinements are disallowed, the only identified benefit of private actual has been removed, and AFAICT, that was the only remaining externally visible difference.

It also seems that the motivating example for the abbreviated syntax (#6347 (comment)) is no longer being addressed.

The principal goal is that when I'm implementing an interface or abstract class, I typically have to write a whole bunch of methods that fill in what are essentially internal implementation details of the class and don't affect the schema of the class from the point of view of a client.

Ceylon has always had a rather verbose syntax for specifying such implementation details, of form:

shared actual ExplicitType<WithArgs> whatever()

Now, shortcut refinement can help a lot, but I can't always use it.

I would like to be able to write such implementation details using a much more streamlined syntax:

actual function whatever()

And I don't see why such "implementation details" need to be considered shared. Clients don't need to depend directly on Subclass.whatever(), they can depend on Superclass.whatever(). So why the requirement for shared in the first place?

Ok.

I don't believe that the rather large proposed patch accomplishes that, whereas 1.3.2 plus a small change to allow shared to be omitted would.

I don't believe that the rather large proposed patch accomplishes that

I don't understand.

Because your motivating examples (which are consistent with your stated goal) will not compile since they involve illegal covariant refinements.

Further, there's still no stated benefit, or even externally visible difference, for implementing non-shared refinements. So the way this is implemented seems to be a non-goal.

I'm not going to spend a lot of time looking for bugs with this patch since I think it would be more efficient to just get rid of it, but I couple things I found when looking at 7003:

interface A {
    shared default String foo() => "";
}
class C() satisfies A {
    actual default String foo() => "";
}

source/simple/run.ceylon:24: error: Ceylon backend error: method foo() is already defined
in class C
    actual default String foo() => "";
           ^

and

interface I {
    shared default String name=> "Not Gavin";
}
interface J satisfies I {
    actual value name => "Gavin";
}
shared void run() {
    print(object satisfies J {}.name); // Not Gavin (should be "Gavin")
}

Ahyes, thanks, I had not tested it with default interface methods.

there's still no stated benefit, or even externally visible difference, for implementing non-shared refinements

I have come up with a user visible functional difference between

actual value foo => "foo";

and

shared actual value foo => "foo";

in that the following does not compile. But I don't see this as a beneficial difference, or even one that any user would notice or prefer on the grounds of purity or correctness if shared was inferred in the implementation of the type checker. I think it would be better for this to compile.

interface I {
    shared formal Object foo;
}

interface J satisfies I {
    shared actual formal String foo;
}

interface K satisfies I {
    shared actual formal String foo;
}

class C() satisfies J & K {
    actual value foo => "foo";
}

shared void run() {
    String s = C().foo; // error
    // specified expression must be assignable to declared type of 's':
    // 'Object' is not assignable to 'String'
}

Since there is no consensus on this yet, and because there are still some bugs related to interface members, not to mention missing tests and spec, this is not for 1.3.3.

xkr47 commented

With my infitesimal amount of voting power I nevertheless vote to

  • not go ahead with hidden refinements
  • not touch shortcut refinements
  • at most do:
shared annotation [SharedAnnotation,ActualAnnotation] actual() =>
        [SharedAnnotation(),ActualAnnotation()];

or if implemented in code, actually set shared = actual = true; in the code so that you don't need to remember to if (shared || actual) in all the places where shared makes a difference.

because

  • I would like to avoid users having a disturbing feeling in the back of their head when something isn't quite working that "maybe adding shared to the hidden refinements could fix the issue" and then just maybe get it to work that way
  • I just don't think this thing is worth a huge diff that potentially makes dev-team development harder and problem-solving nastier
  • as long as you don't have hidden refinements, actual is not orthogonal with shared in my opinion

If I may have a say in this, IMO I believe:

  • a shared formal member shouldn't have an un-shared refinement, because the subtype is ultimately a set containing the one representing its supertype, right? And their views shouldn't be altered because it may confuse the user (it may not be immediately clear to the common user);
  • an un-shared member shouldn't have an un-shared refinement, because it is not visible to the subtype, maybe what you want is just an extension of a supertype's internal property or behaviour.

That is, since having an un-shared member refining an un-shared refinement is not good, it would be useful to have another annotation, that can be applied on supertype's members, which defines inheritable internal ("un-shared") properties or behaviours, e.g.

class A() {
    inheritable void internalBehaviour() { ... }
}
class B() extends A() {
    [inheritable] actual void internalBehaviour() { ... }
}

Obviously, using both shared and inheritable may be allowed, but it would be the same as using shared (the most widening one, that is if it's shared then it's inheritable).
As of now, this is the only thing it makes sense to me in allowing "un-shared" refinements.