static methods and attributes

gavinking commented 8 years ago

So, I'm throwing this one out there to see what ya'll think.

A long time ago I decided that Ceylon would not have static methods. The reason for this was that I always hated them in Java: in order to declare a regular function, you were forced to define a rubbish class with no instances, just to stick the function on it, and then to call that function, you had to do something nastily verbose like Integer.parseInt(). These functions had nothing to do with the schema of any type, and they don't have access to the instance-level state of the class, they are just stuck onto the side of a class for no good reason.

Some things happened since that time to make me want to reevaluate this:

We had to anyway build almost everything needed to support static methods into the typechecker in order to support Java interop.
We added constructors, which sorta look like static methods. In particular, they are invoked on a class name, instead of an instance.
People started asking for "factory functions", which are, essentially, a particular sort of static method.
I figured out how to resolve the "don't have access to the instance-level state" conundrum: simply require that static members be declared at the beginning of a class with constructors, before any instance-level state is allocated.
I started writing lots of code that interoperates Java with Ceylon and found it sorta weird that I was totally comfortable calling Java static methods in Ceylon, but couldn't actually write a static method in Ceylon.
I started wanting to define a constant values shared between all instances of a class, without polluting the package namespace.
I ran into a (anti-)pattern used in IntelliJ APIs where a service implementation is supposed to define a certain specially-named static member. I couldn't do this in Ceylon, so this class had to be written in Java.
I realized that there actually is one very good motivation for static: static members can bypass the visibility restrictions that are imposed on toplevel functions, and access private state of instances of the class. This is unambiguously useful, especially since we don't (yet) have package-private.

Thus, at this point, it would be sooo easy to add static methods and attributes (it basically amounts to one new annotation), that it's hard to see why we shouldn't. It would look like this:

class Integer {

    shared static Integer|SyntaxError parse(String string) => internalParse(string);
    shared static Integer sum({Integer*} integers) => integers.fold(0)((s,x)=>s+x);

    shared new (Integer i) { ... }

    ...

}

Note that I don't see why interfaces couldn't have static members. So this need not just be for classes.

So, pros and cons of doing this include:

Pros:

it makes it slightly easier to port Java code to Ceylon (paste-Java-as-Ceylon will work a little better)
it results in the more-expressive Integer.parse(text) and Integer.sum(totals) instead of parseInteger(text) and integerSum(totals)
it's easier to "discover" Integer.parse() and Integer.sum() in the IDE than it is to discover parseInteger() today
it provides a nice way to share private state/operations between a class and functions which operate on its instances,
it provides a way to define class-local constants,
it probably resolves the perceived need for "factory functions"
it makes constructors look less "irregular", since static members and constructors share a essentially the same rules
it slightly eases the cognitive tax of programming in mixed Java/Ceylon projects, since that's one more construct that maps across cleanly (ditto for JavaScript where you have stuff like Math.xxxx)
it might make some things from Dart and Typescript easier to represent @jvasileff @lucaswerkmeister wdyt?

Cons:

it might encourage Java-isms of creating rubbish classes just to stick functions on them (but it's easy enough to tell people to use object instead of class for that
the people (person?) who think(s) that introducing constructors were a terrible mistake that makes the language less regular might perceive this as doubling down on that irregularity rather than resolving it by make it less irregular
it's an additional language feature that is clearly not a critical thing we have to have
it might be more difficult to implement in Dart and JS than it is in Java (I doubt this)
I would have to admit I was wrong about something

Thoughts?

gavinking commented 8 years ago

@lucaswerkmeister yes, that, or:

shared interface Summable<Other> of Other ... {
    shared formal static Other additiveIdentity;
    ...
}
shared native final class Integer(Integer integer) ... {
    shared actual static Integer additiveIdentity = 0;
    ...
}

I'm allowing static constants in generic types. Just not static variables. Furthermore, Integer is not generic.

gavinking commented 8 years ago

@ePaul your argument has shades of "class variables" vs "class instance variables" in Smalltalk :)

tombentley commented 8 years ago

@gavinking static member classes with generic outers should not capture the outer's type parameter, but I still need to fix the same thing for static values and functions.

gavinking commented 8 years ago

"Should now"?

tombentley commented 8 years ago

Should now indeed

On 30 Sep 2016 17:31, "Gavin King" notifications@github.com wrote:

"Should now"?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/ceylon/ceylon/issues/6515#issuecomment-250790774, or mute the thread https://github.com/notifications/unsubscribe-auth/AA1rf3JfONuZfFSBDE9lYoeAcXrUobI5ks5qvTl7gaJpZM4KBfS6 .

gavinking commented 8 years ago

@tombentley with this code:

class Class<T> {
    shared static class Inner(T t) {
        shared T get() => t;
    }
    shared new () {}
}

I still get:

Error:(2, 31) ceylon: Ceylon backend error: non-static type variable T cannot be referenced from a static context
Error:(2, 12) ceylon: Ceylon backend error: non-static type variable T cannot be referenced from a static context
Error:(3, 9) ceylon: Ceylon backend error: non-static type variable T cannot be referenced from a static context
Error:ceylon: Ceylon backend error: non-static variable this cannot be referenced from a static context

I might be doing something wrong.

tombentley commented 8 years ago

@gavinking try now (I forgot to push, sorry).

gavinking commented 8 years ago

@tombentley thanks, it's better, but now, for this code:

class Class<T> {
    shared static class Inner(T t) {
        shared T get() => t;
    }
    shared new () {}
}

shared void run() {
    Character character
            = Class<Character>().Inner('X').get();
    print(character);
}

I get:

Error:(10, 48) ceylon: Ceylon backend error: cannot find symbol
  symbol:   method intValue()
  location: class java.lang.Object
Error:(10, 40) ceylon: Java primitive boxing/unboxing in Ceylon-generated code!

jvasileff commented 8 years ago

This is a bug of one form or another:

class X {
    shared static Integer i() => 1;
    shared new () {
        print("newx");
    }
}
shared void run() {
    noop(X().i()); // prints nothing
}

Either qualifying the static member with an instance should be disallowed (my preference), or the program should print "newx".

This may also be an interop bug affecting 1.3.0, but I didn't test that.

chochos commented 8 years ago

I agree. Static methods should be properly qualified. I've always disliked that feature in Java, that you can reference/call static stuff as if it were part of an instance.

lucaswerkmeister commented 8 years ago

Even within non-static members of the own class?

tombentley commented 8 years ago

@gavinking your problem should be fixed, and we now have support for type parameter capture by methods and attributes.

gavinking commented 8 years ago

Excellent! Thanks

gavinking commented 8 years ago

@chochos where are you at on this?

gavinking commented 8 years ago

@chochos on the branch I now get a bunch of errors like this:

     [java] Compiling language module sources: process.js
     [java] Compiling language module sources: .
     [java] ../language/src/ceylon/language/formatFloat.ceylon:256: error: the 'GenerateJsVisitor' caused an exception visiting a 'QualifiedMemberExpression' node: '"java.lang.NullPointerException"' at 'com.redhat.ceylon.compiler.js.GenerateJsVisitor.visit(GenerateJsVisitor.java:1947)'
     [java]         Float n = Math.log(num);
     [java]                        ^
     [java] ../language/src/ceylon/language/formatFloat.ceylon:257: error: the 'GenerateJsVisitor' caused an exception visiting a 'QualifiedMemberExpression' node: '"java.lang.NullPointerException"' at 'com.redhat.ceylon.compiler.js.GenerateJsVisitor.visit(GenerateJsVisitor.java:1947)'
     [java]         Float d = Math.\iLN10;
     [java]                        ^

tombentley commented 8 years ago

@gavinking before I waste time fixing it, can you confirm that you do wish to support instance-qualified static member access?

gavinking commented 8 years ago

@tombentley don't waste time on it right now, please.

gavinking commented 8 years ago

I just got this error while trying to build IntelliJ on the branch:

[ceylon-compile] /Users/gavin/ceylon-ide-common/source/com/redhat/ceylon/ide/common/model/CeylonProject.ceylon:302: error: Ceylon backend error: method allOf in class EnumSet<E#2> cannot be applied to given types;
[ceylon-compile]             configuration.suppressWarningsEnum != EnumSet.allOf(javaClass<Warning>());
[ceylon-compile]                                                   ^
[ceylon-compile]   required: Class<E#1>
[ceylon-compile]   found: Class<Warning>
[ceylon-compile]   reason: actual and formal argument lists differ in length
[ceylon-compile]   where E#1,E#2 are type-variables:
[ceylon-compile]     E#1 extends Enum<E#1> declared in method <E#1>allOf(Class<E#1>)
[ceylon-compile]     E#2 extends Enum<E#2> declared in class EnumSet
[ceylon-compile] /Users/gavin/ceylon-ide-common/source/com/redhat/ceylon/ide/common/model/CeylonProject.ceylon:302: error: Ceylon backend error: method invoked with incorrect number of arguments; expected 1, found 0
[ceylon-compile]             configuration.suppressWarningsEnum != EnumSet.allOf(javaClass<Warning>());
[ceylon-compile]                                                                ^
[ceylon-compile] /Users/gavin/ceylon-ide-common/source/com/redhat/ceylon/ide/common/model/CeylonProject.ceylon:302: error: Java primitive boxing/unboxing in Ceylon-generated code!
[ceylon-compile]             configuration.suppressWarningsEnum != EnumSet.allOf(javaClass<Warning>());
[ceylon-compile]             ^

I'm guessing that this has something to do with the changes @tombentley made? WDYT, Tom?

chochos commented 8 years ago

It would be very helpful to have some common tests for all backends on this, to make sure everything works the same in all backends.

jvasileff commented 8 years ago

@chochos far from comprehensive, but if it helps, here's what I used: https://gist.github.com/jvasileff/8b15c2dcb0fd06bc0871eb7ba87dbc85

chochos commented 8 years ago

Damn, these SomeClass().staticMember calls are also very troublesome in the JS backend. Why are they even allowed? I feel weird just writing that code.

tombentley commented 8 years ago

@gavinking intellij problem fixed.

gavinking commented 8 years ago

Thank you sir.

gavinking commented 8 years ago

qualifying the static member with an instance should be disallowed

It is now rejected by the typechecker.

chochos commented 8 years ago

OK so I got invocations working, and static classes and interfaces as well. What are the changes to the metamodel? Are there tests in place yet? I mean I can see that they break right now but some of those seem to be because they're not valid anymore (like the one on Integer parameter, since now Integer has a constructor, to accommodate for a static method that I don't even know why it's static in the first place).

tombentley commented 8 years ago

I've not touched the metamodel yet.

On 7 Oct 2016 9:24 p.m., "Enrique Zamudio" notifications@github.com wrote:

OK so I got invocations working, and static classes and interfaces as well. What are the changes to the metamodel? Are there tests in place yet? I mean I can see that they break right now but some of those seem to be because they're not valid anymore (like the one on Integer parameter, since now Integer has a constructor, to accommodate for a static method that I don't even know why it's static in the first place).

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/ceylon/ceylon/issues/6515#issuecomment-252352365, or mute the thread https://github.com/notifications/unsubscribe-auth/AA1rf80nnYa-CUn1u1qNDn8g19Q50SLvks5qxqqAgaJpZM4KBfS6 .

gavinking commented 8 years ago

I have not touched the metamodel either. We need to think it through, I suppose. But I'm inclined to think we're ready to merge it now. Any objections?

tombentley commented 8 years ago

I agree, merge it.

On 8 Oct 2016 6:06 p.m., "Gavin King" notifications@github.com wrote:

I have not touched the metamodel either. We need to think it through, I suppose. But I'm inclined to think we're ready to merge it now. Any objections?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/ceylon/ceylon/issues/6515#issuecomment-252436258, or mute the thread https://github.com/notifications/unsubscribe-auth/AA1rf6mn2ieToDvkVBIzULWXGgfG8IERks5qx82ygaJpZM4KBfS6 .

gavinking commented 8 years ago

I have merged the branch.

gavinking commented 8 years ago

@chochos I have this code:

class Hello {
    static shared String name = "Gavin";
    shared static class Goodbye() {}
    shared new () {}
}

shared void run() {
    Hello.Goodbye() name = Hello.Goodbye;
    Hello.Goodbye goodbye = name();
}

And I get this error when running it on JS:

/Users/gavin/untitled/modules/foo/bar/1.0.0/foo.bar-1.0.0.js:36
if(goodbye$Hello$===undefined)goodbye$Hello$=new this.Goodbye$Hello.$$;
                                                                   ^

TypeError: Cannot read property '$$' of undefined
    at Goodbye$Hello (/Users/gavin/untitled/modules/foo/bar/1.0.0/foo.bar-1.0.0.js:36:68)
    at c2 (/Users/gavin/Library/Application Support/IdeaIC2016.2/CeylonIDEA/classes/embeddedDist/repo/ceylon/language/1.3.1-SNAPSHOT/ceylon.language-1.3.1-SNAPSHOT.js:724:11)
    at run (/Users/gavin/untitled/modules/foo/bar/1.0.0/foo.bar-1.0.0.js:56:8)
    at [eval]:1:285

gavinking commented 8 years ago

So, thinking about the metamodel a bit.

declaration refs

Declaration refs like value Hello.name and function Hello.hello are the easy case. I don't need to make any changes to typechecker, AFAICS, but we need to adjust the implementation so that the following work:

value name = `value Hello.name`.get();
`function Hello.hello`.invoke();

And I guess we also need to add an attribute NestableDeclaration.static.

model references

We could introduce a whole new parallel hierarchy hanging off a new StaticMember type, but that seems like a bit too much work. So an alternative would be to make static models instances of Member<Null>. For example, Hello.name would be an Attribute<Null, String>. And you could write:

value name = `Hello.name`(null).get();

This would require a pretty small change to the typechecker.

Alternatively, I suppose we could make it an Attribute<Class<Hello>,String>, and make you write:

value name = `Hello.name`(`Hello`).get();

But TBH I can't see any advantage to that, at least not until we have the type class stuff.

gavinking commented 8 years ago

This would require a pretty small change to the typechecker.

I have made that change, but I'm not attached to this solution.

tombentley commented 8 years ago

make static models instances of Member<Null>

Well this is attractive because it's simple (and familiar wrt to Java reflection APIs), but it does make it look like these things are instance methods of Null. I can imagine people writing code which inspected container types and assuming that the container type actually had these methods. But really that's a minor gripe because I have no appetite for a whole new hierarchy.

jvasileff commented 8 years ago

This breaks the typechecker:

class C {
    shared static String val => "val";
    shared static String fun() => "fun";
    noop([C()]*.val);
    noop([C()]*.fun());
    shared new () {}
}

gavinking commented 8 years ago

@jvasileff thanks, fixed.

gavinking commented 8 years ago

@chochos we still need implementations of Integer/Float.parse/format() for the JS backend.

chochos commented 8 years ago

methods for Integer were already implemented, but Float were missing.

BTW Integer.parse("foo") does not return a ParseException on JVM.

gavinking commented 8 years ago

@chochos thanks, fixed.

gavinking commented 7 years ago

@tombentley I found a code example that reveals multiple backend bugs related to static:

import javax.enterprise.context {
    requestScoped
}
import javax.enterprise.inject {
    produces
}
import javax.persistence {
    persistenceContext,
    EntityManager
}

requestScoped
shared class PersistenceProvider {

    suppressWarnings("unusedDeclaration")
    produces persistenceContext
    static late EntityManager entityManager;

    shared new () {
    }
}

Currently this completely blows up the compiler. Removing the suppressWarnings("unusedDeclaration") will reveal a different bug.

tombentley commented 7 years ago

Fixed @gavinking.

I guess now that we've merged the branch we should close this issue now, no?

gavinking commented 7 years ago

@tombentley Well, I guess I thought we were still waiting for metamodels?

gavinking commented 7 years ago

Closing this now that #6629 covers the metamodel.

xkr47 commented 7 years ago

Crazy stuff. Just when I felt totally comfy with not using static.. But I can't argue much with @gavinking's initial comment :)

eclipse-archived / ceylon

static methods and attributes #6515

declaration refs

model references