ceylon / ceylon-spec

DEPRECATED
Apache License 2.0
108 stars 34 forks source link

lazy specification of `void` methods #479

Closed tombentley closed 11 years ago

tombentley commented 11 years ago

@FroMage spotted this in the compiler tests:

void methodSpecifyingUnaryTopLevel_f(String s) {
}
class MethodSpecifyingUnaryTopLevel() {
    void foo(String s) => methodSpecifyingUnaryTopLevel_f;
}

I forgot to fix the lazy specification (it should be => methodSpecifyingUnaryTopLevel_f(s)), but I'm surprised the typechecker permits this when the analogous:

class MethodSpecifyingUnaryTopLevel() {
    void foo(String s) {
        return methodSpecifyingUnaryTopLevel_f;
    }
}

is not allowed. Is there a reason for this, or is it a bug?

gavinking commented 11 years ago

well that's a good question. it's not a bug but perhaps we should disallow it anyway, on the basis that it is almost always a bug in the code.

the problem is that to me it seemed a little heavy handed to completely disallow => for void methods but, well, perhaps it's ok after all.

Sent from my iPhone

On 29/11/2012, at 3:31 PM, Tom Bentley notifications@github.com wrote:

@FroMage spotted this in the compiler tests:

void methodSpecifyingUnaryTopLevel_f(String s) { } class MethodSpecifyingUnaryTopLevel() { void foo(String s) => methodSpecifyingUnaryTopLevel_f; } I forgot to fix the lazy specification (it should be => methodSpecifyingUnaryTopLevel_f(s)), but I'm surprised the typechecker permits this when the analogous:

class MethodSpecifyingUnaryTopLevel() { void foo(String s) { return methodSpecifyingUnaryTopLevel_f; } } is not allowed. Is there a reason for this, or is it a bug?

— Reply to this email directly or view it on GitHub.

FroMage commented 11 years ago

the problem is that to me it seemed a little heavy handed to completely disallow => for void methods but, well, perhaps it's ok after all.

Wouldn't that mean disallowing function () print("foo") too?

gavinking commented 11 years ago

grr. that would seem the consistent thing to do. :(

Sent from my iPhone

On 29/11/2012, at 3:48 PM, Stéphane Épardaud notifications@github.com wrote:

the problem is that to me it seemed a little heavy handed to completely disallow => for void methods but, well, perhaps it's ok after all.

Wouldn't that mean disallowing function () print("foo") too?

— Reply to this email directly or view it on GitHub.

quintesse commented 11 years ago

I agree it seems heavy-handed, but I must also say it will prevent a class of easily overlooked very confusing errors.

You will never want to do void foo(String s) => methodSpecifyingUnaryTopLevel_f; and void foo(String s) => methodSpecifyingUnaryTopLevel_f(s); is easily rewritten as void foo(String s) { methodSpecifyingUnaryTopLevel_f(s); }

gavinking commented 11 years ago

OK, so here's what we could do:

This would be completely self-consistent.

gavinking commented 11 years ago

So I suppose we're doing this one? i.e. disallowing => after void in both declared and anonymous functions? Anybody disagree with it?

gavinking commented 11 years ago

Done in the branch.

gavinking commented 11 years ago

So are we also going to disallow this:

void f();
f() => "hello";

?

gavinking commented 11 years ago

For consistency, that also meant disallowing => in arguments to void-declared functional parameters. Don't like that much at all. Now there is no way to specify a default argument to a void-declared functional parameter. :-/

chochos commented 11 years ago

There will be after merging the multiline lambda branch...

gavinking commented 11 years ago

There will be after merging the multiline lambda branch...

You want to be able to write:

function do(void task() {}) { do(); }

i.e a block of statements nested inside a parameter list? Yew!

ikasiuk commented 11 years ago

Can you at least write

void noop() {}
void do(Void() task=noop) { task(); }

or something like that?

gavinking commented 11 years ago

No. We eliminated that syntax when we introduced the fat arrow stuff.

ikasiuk commented 11 years ago

No. We eliminated that syntax when we introduced the fat arrow stuff.

Actually, that noop example still seems to work with the current version?! What part of the syntax exactly did you eliminate?

I'd expect the case of a void function parameter with a no-op default to be rather common (required more or less for any kind of optional callback). So we should provide a way to express that.

gavinking commented 11 years ago

Ah, then that's a bug.

Note that you can express it like this:

Void callback()=>null

Or like this:

Void() callback=(){}

Ugly.

ikasiuk commented 11 years ago

So you can write Void() callback=(){} but Void() task=noop is a bug? Why?

gavinking commented 11 years ago

Actually, that noop example still seems to work with the current version?!

Ah, then that's a bug.

I'm sorry, I misread what you typed. (I was reading on my phone.)

Yes, you can write that.

gavinking commented 11 years ago

Closing. If this turns out to be bad in practice, we can revisit it.

FroMage commented 11 years ago

Really not sure about that one, I thought it was better before.

gavinking commented 11 years ago

Actually I think we could loosen the restriction slightly. I think we should let you alias an invocation of a void method if the function is declared void, i.e. I think the following should be accepted:

void f(String s) {}
void g(String s1, String s2) => f(s1+s2);
gavinking commented 11 years ago

OK, I'm now letting void functions alias void functions, which solves the problem with this, I think. For example, you can write:

function f(void g()=>print("hello")) { ... }

(Because print() is declared void.)

I'm leaving this open for now because I need to deal with the TODO at ExpressionVisitor.java:2171.

FroMage commented 11 years ago

It is better, but I predict the second most popular user function (after eq) will be void ignoreValueYouMotherFucker(Anything whatever){} to let you write void f() => ignoreValueYouMotherFucker(functionThatReturnsAValue()). I suggest to drop the constraint entirely.

gavinking commented 11 years ago

I definitely can't drop the constraint entirely, because I think it's clear that the following should be disallowed:

void doSomething() => 1+1;

An expression is not a valid statement.

FroMage commented 11 years ago

Right, maybe an expression is not valid, but a statement expression such as functionThatReturnsSomething() should be.

gavinking commented 11 years ago

@FroMage you might be right, but let's see how we go with this how it is right now—we can always come back later and remove restrictions. It's adding new restrictions that is more problematic.

FroMage commented 11 years ago

Looks like we can still not specify void methods: cannot specify void method: print1

FroMage commented 11 years ago

At the risk of being yelled at some more, I'm sorry to say there are still corner cases you haven't fixed:

/*
 * Copyright Red Hat Inc. and/or its affiliates and other contributors
 * as indicated by the authors tag. All rights reserved.
 *
 * This copyrighted material is made available to anyone wishing to use,
 * modify, copy, or redistribute it subject to the terms and conditions
 * of the GNU General Public License version 2.
 * 
 * This particular file is subject to the "Classpath" exception as provided in the 
 * LICENSE file that accompanied this code.
 * 
 * This program is distributed in the hope that it will be useful, but WITHOUT A
 * WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS FOR A
 * PARTICULAR PURPOSE.  See the GNU General Public License for more details.
 * You should have received a copy of the GNU General Public License,
 * along with this distribution; if not, write to the Free Software
 * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston,
 * MA  02110-1301, USA.
 */
@nomodel
interface MethodRefinementWithSpecifiers_Interface {
    shared formal void f2();
    shared formal void f3();
    shared formal void f4();
    shared formal void f5();
    shared formal void f6();
    shared formal void f7();
    shared formal void f8();
    shared formal void f9();
    shared formal void f10();
    shared formal Object f11();
    shared formal Object f12();
    shared formal Object f13();
    shared formal void f14(Integer i);
    shared formal void f15(Integer i);
    shared formal Object f16();
    shared formal Object f17();
    shared formal Object f18();
    shared formal Object f19();
}

@nomodel
void methodRefinementWithSpecifiers_returnsMethod()(){}

@nomodel
Callable<Anything,[]> methodRefinementWithSpecifiers_attributeMethod = methodRefinementWithSpecifiers_returnsMethod();

@nomodel
class MethodRefinementWithSpecifiers_ClassNoParam(){}

@nomodel
class MethodRefinementWithSpecifiers_Class(void arg()) satisfies MethodRefinementWithSpecifiers_Interface {

    class Inner(){}
    shared class InnerShared(){}

    // long decl, lazy, arg always reevaluated
    shared actual void f2() => arg();

    // short decl, eager, but arg is constant so it's optimised
    f3 = arg;

    // short decl, lazy, arg always reevaluated
    f4() => arg();

    // short decl, lazy, arg always reevaluated
    f5 = function () => arg();

    // long decl, lazy, arg always reevaluated
    shared actual void f6(){
        arg();
    }

    // short decl, eager, but f3 is constant so it's optimised
    f7 = f3;

    // short decl, lazy, f3 always reevaluated
    f8() => f3();

    // short decl, eager, returnsMethod() is only evaluated once
    f9 = methodRefinementWithSpecifiers_returnsMethod();

    // short decl, eager, attributeMethod is only evaluated once
    f10 = methodRefinementWithSpecifiers_attributeMethod;

    // short decl, eager, but Bug891_ClassNoParam is constant to it's optimised
    f11 = MethodRefinementWithSpecifiers_ClassNoParam;

    // short decl, eager, but Inner is constant to it's optimised
    f12 = Inner;

    // short decl, eager, but InnerShared is constant to it's optimised to its instantiator method
    f13 = InnerShared;

    void takesParam(Integer i){}

    // short decl, eager, but takesParam is constant to it's optimised
    f14 = takesParam;

    // short decl, eager, but f14 is constant to it's optimised
    f15 = f14;

    // short decl, lazy, returns a Callable
    f16() => takesParam;

    // short decl, lazy, returns a Callable
    f17() => Inner;

    // short decl, lazy, returns a Callable
    f18() => arg;

    // long decl, lazy, returns a Callable
    shared actual Object f19() => arg;
}

Produces:

test/src/com/redhat/ceylon/compiler/java/test/structure/method/MethodRefinementWithSpecifiers.ceylon:58:ERROR:void method may not evaluate to a value
test/src/com/redhat/ceylon/compiler/java/test/structure/method/MethodRefinementWithSpecifiers.ceylon:64:ERROR:method is declared void so specified expression may not evaluate to a value: f4
FroMage commented 11 years ago

Thanks, that fixed it!

ikasiuk commented 11 years ago

Syntax question: I have some old code like the following

class MyCache(callback=(String k) defaultAction(k)) satisfies Cache {
    doc "bla bla"
    shared actual void callback(String key);
    //...
}

How is this written correctly with the current syntax?

quintesse commented 11 years ago

I don't know if it's sufficient but you'll need at least the fat arrow:

class MyCache(callback=(String k) => defaultAction(k)) satisfies Cache {
    doc "bla bla"
    shared actual void callback(String key);
    //...
}
ikasiuk commented 11 years ago

Yes, that seems to work. Thank you!

gavinking commented 11 years ago

Right, the fat arrow is now required, since we added support for multi-statement anon functions. It's needed to disambiguate the {. Anyway it makes the language more regular. And now that I've got used to it I actually find anonymous functions easier to read with it in.

ikasiuk commented 11 years ago

Yes, I agree. Just lost track of the syntactic details a bit after the latest changes :-)