uqbar-project / wollok

Wollok Programming Language
GNU General Public License v3.0
61 stars 16 forks source link

method lookup fails on mixins with abstract method #1002

Open lgassman opened 7 years ago

lgassman commented 7 years ago

See this example:

mixin Mixin1 {  
    method m1() {
        return super()
    }
}

mixin Mixin2 {
    method m2() {
        return self.m1()
    }
    method m1() 
}

class SuperClass {
    method m1() {
        return "SuperClass.m1"
    }
}

class SubClass inherits SuperClass mixed with Mixin2, Mixin1{   
}

REPL history:


Consola Interactiva de Wollok (escriba "quit" para salir): 
>>> new SubClass().m1()
     Error de la VM de Wollok en la linea (1): new SubClass().m1():
          Error de la VM de Wollok en la linea (137): {
        return super()
    }:
               Error de la VM de Wollok en la linea (138): return super():
                    Error de la VM de Wollok en la linea (138): super():
java.lang.NullPointerException
    at org.uqbar.project.wollok.interpreter.WollokInterpreterException.createMessage(WollokInterpreterException.java:59)
    at org.uqbar.project.wollok.interpreter.WollokInterpreterException.<init>(WollokInterpreterException.java:35)
    at org.uqbar.project.wollok.interpreter.WollokInterpreter.eval(WollokInterpreter.java:252)
    at org.uqbar.project.wollok.interpreter.AbstractWollokCallable.eval(AbstractWollokCallable.java:133)
    at org.uqbar.project.wollok.interpreter.AbstractWollokCallable$1.apply(AbstractWollokCallable.java:88)
    at org.uqbar.project.wollok.interpreter.AbstractWollokCallable$1.apply(AbstractWollokCallable.java:1)
    at org.uqbar.project.wollok.interpreter.WollokInterpreter.performOnStack(WollokInterpreter.java:197)
    at org.uqbar.project.wollok.interpreter.AbstractWollokCallable.call(AbstractWollokCallable.java:101)
    at org.uqbar.project.wollok.interpreter.core.WollokObject.callSuper(WollokObject.java:514)
    at org.uqbar.project.wollok.interpreter.core.CallableSuper.call(CallableSuper.java:29)
    at org.uqbar.project.wollok.interpreter.WollokInterpreterEvaluator._evaluate(WollokInterpreterEvaluator.java:832)
    at org.uqbar.project.wollok.launch.WollokLauncherInterpreterEvaluator.evaluate(WollokLauncherInterpreterEvaluator.java:211)
    at org.uqbar.project.wollok.interpreter.WollokInterpreterEvaluator.evaluate(WollokInterpreterEvaluator.java:1)
    at org.uqbar.project.wollok.interpreter.WollokInterpreter.eval(WollokInterpreter.java:240)
    at org.uqbar.project.wollok.interpreter.WollokInterpreterEvaluator.eval(WollokInterpreterEvaluator.java:133)
    at org.uqbar.project.wollok.interpreter.WollokInterpreterEvaluator._evaluate(WollokInterpreterEvaluator.java:521)
    at org.uqbar.project.wollok.launch.WollokLauncherInterpreterEvaluator.evaluate(WollokLauncherInterpreterEvaluator.java:191)
    at org.uqbar.project.wollok.interpreter.WollokInterpreterEvaluator.evaluate(WollokInterpreterEvaluator.java:1)
    at org.uqbar.project.wollok.interpreter.WollokInterpreter.eval(WollokInterpreter.java:240)
    at org.uqbar.project.wollok.interpreter.WollokInterpreterEvaluator.eval(WollokInterpreterEvaluator.java:133)
    at org.uqbar.project.wollok.interpreter.WollokInterpreterEvaluator$1.apply(WollokInterpreterEvaluator.java:140)
    at org.uqbar.project.wollok.interpreter.WollokInterpreterEvaluator$1.apply(WollokInterpreterEvaluator.java:1)
    at org.eclipse.xtext.xbase.lib.IteratorExtensions.fold(IteratorExtensions.java:627)
    at org.eclipse.xtext.xbase.lib.IterableExtensions.fold(IterableExtensions.java:536)
    at org.uqbar.project.wollok.interpreter.WollokInterpreterEvaluator.evalAll(WollokInterpreterEvaluator.java:143)
    at org.uqbar.project.wollok.interpreter.WollokInterpreterEvaluator._evaluate(WollokInterpreterEvaluator.java:668)
    at org.uqbar.project.wollok.launch.WollokLauncherInterpreterEvaluator.evaluate(WollokLauncherInterpreterEvaluator.java:171)
    at org.uqbar.project.wollok.interpreter.WollokInterpreterEvaluator.evaluate(WollokInterpreterEvaluator.java:1)
    at org.uqbar.project.wollok.interpreter.WollokInterpreter.eval(WollokInterpreter.java:240)
    at org.uqbar.project.wollok.interpreter.AbstractWollokCallable.eval(AbstractWollokCallable.java:133)
    at org.uqbar.project.wollok.interpreter.AbstractWollokCallable$1.apply(AbstractWollokCallable.java:88)
    at org.uqbar.project.wollok.interpreter.AbstractWollokCallable$1.apply(AbstractWollokCallable.java:1)
    at org.uqbar.project.wollok.interpreter.WollokInterpreter.performOnStack(WollokInterpreter.java:197)
    at org.uqbar.project.wollok.interpreter.AbstractWollokCallable.call(AbstractWollokCallable.java:101)
    at org.uqbar.project.wollok.interpreter.core.WollokObject.call(WollokObject.java:122)
    at org.uqbar.project.wollok.interpreter.WollokInterpreterEvaluator._evaluate(WollokInterpreterEvaluator.java:832)
    at org.uqbar.project.wollok.launch.WollokLauncherInterpreterEvaluator.evaluate(WollokLauncherInterpreterEvaluator.java:211)
    at org.uqbar.project.wollok.interpreter.WollokInterpreterEvaluator.evaluate(WollokInterpreterEvaluator.java:1)
    at org.uqbar.project.wollok.interpreter.WollokInterpreter.eval(WollokInterpreter.java:240)
    at org.uqbar.project.wollok.interpreter.WollokInterpreterEvaluator.eval(WollokInterpreterEvaluator.java:133)
    at org.uqbar.project.wollok.interpreter.WollokInterpreterEvaluator$1.apply(WollokInterpreterEvaluator.java:140)
    at org.uqbar.project.wollok.interpreter.WollokInterpreterEvaluator$1.apply(WollokInterpreterEvaluator.java:1)
    at org.eclipse.xtext.xbase.lib.IteratorExtensions.fold(IteratorExtensions.java:627)
    at org.eclipse.xtext.xbase.lib.IterableExtensions.fold(IterableExtensions.java:536)
    at org.uqbar.project.wollok.interpreter.WollokInterpreterEvaluator.evalAll(WollokInterpreterEvaluator.java:143)
    at org.uqbar.project.wollok.interpreter.WollokInterpreterEvaluator._evaluate(WollokInterpreterEvaluator.java:188)
    at org.uqbar.project.wollok.launch.WollokLauncherInterpreterEvaluator.evaluate(WollokLauncherInterpreterEvaluator.java:215)
    at org.uqbar.project.wollok.interpreter.WollokInterpreterEvaluator.evaluate(WollokInterpreterEvaluator.java:1)
    at org.uqbar.project.wollok.interpreter.WollokInterpreter.eval(WollokInterpreter.java:240)
    at org.uqbar.project.wollok.interpreter.WollokInterpreterEvaluator.eval(WollokInterpreterEvaluator.java:133)
    at org.uqbar.project.wollok.launch.WollokLauncherInterpreterEvaluator._evaluate(WollokLauncherInterpreterEvaluator.java:69)
    at org.uqbar.project.wollok.launch.WollokLauncherInterpreterEvaluator.evaluate(WollokLauncherInterpreterEvaluator.java:213)
    at org.uqbar.project.wollok.interpreter.WollokInterpreterEvaluator.evaluate(WollokInterpreterEvaluator.java:1)
    at org.uqbar.project.wollok.interpreter.WollokInterpreter.interpret(WollokInterpreter.java:153)
    at org.uqbar.project.wollok.launch.repl.WollokRepl.executeInput(WollokRepl.java:168)
    at org.uqbar.project.wollok.launch.repl.WollokRepl.startRepl(WollokRepl.java:90)
    at org.uqbar.project.wollok.launch.WollokLauncher.doSomething(WollokLauncher.java:50)
    at org.uqbar.project.wollok.launch.WollokChecker.doMain(WollokChecker.java:83)
    at org.uqbar.project.wollok.launch.WollokLauncher.main(WollokLauncher.java:34)
javierfernandes commented 7 years ago

There is probably a first bug (NPE) while trying to show an error. Then the error itself. I guess that it is that it tried to execute an abstract method.

Now besides that, there is a semantic decision here. Basically this shows that today you can come up with a hierarchy chain like this (regarding method overwrite)

concrete <---  abstract <--- concrete( with super)

The question are

The latest is actually an idea that we have been discussing for some time when defining mixins. m1() in Mixin2 could be thought as a "method requirement". And not as "I'm defining a method". So in that "sense" it shouldn't be taken into account for the lookup. One solution would be to differentiate this ideas syntactically:

mixin Mixin2 {

     requires method m1()      // this is not a method definition, just a constrain

     method m5()                    // should we still allow abstract methods definitions ?

}

None of this approaches have relative common ideas on other industrial languages, I guess. But it shouldn't matter if they are consistent. Scala for examples introduces also their not-so-natural ideas like the fact that a mixin can extends a class or a mixin, or "abstract override" keywords o typing "this". They are all cases to impose requirements over where the mixin can be combined.

npasserini commented 7 years ago

I kind of like the scala semantics in this point. When I have a mixin with abstract m1 it should be read "I must be installed in an object with a defined m1". We should not care if it is after or before in the hierarchy. I think it is very powerful.

This leads to several cases. Following Leo's code

object x = new SuperClass mixed with M1 // Ok object y = new Object mixed with M1 // Not ok, Object does not define m1() object z = new Object mixed with M1, M2 // Ok, M2 fills the requierement.

class C extends Object mixed with M1 {} // Also ok, but C is an abstract class

I think that we can avoid having two different cases:

 requires method m1()      // this is not a method definition,

just a constrain

 method m5()                    // should we still allow abstract

methods definitions ?

We could have just one, you have a requirement and you do not care where it is fullfilled.

On Sat, Oct 1, 2016 at 4:02 PM, javierfernandes notifications@github.com wrote:

There is probably a first bug (NPE) while trying to show an error. Then the error itself. I guess that it is that it tried to execute an abstract method.

Now besides that, there is a semantic decision here. Basically this shows that today you can come up with a hierarchy chain like this (regarding method overwrite)

concrete <--- abstract <--- concrete( with super)

The question are

  • Should we consider the abstract as meaning "I'm cancelling the method implementation forcing subclasses to implement" ?
  • If "yes", then I guess that the hierarchy should be detected as invalida, but just because of using "super" in a mixin, having an abstract impl as the first super.
  • It could be "no" then it could be at least two cases: 1) just ignoring the abstract method in the lookup (which seems also weird), 2) analyze the "abstract method declaration" REAL INTENTION in the mixin.

The latest is actually an idea that we have been discussing for some time when defining mixins. m1() in Mixin2 could be thought as a "method requirement". And not as "I'm defining a method". So in that "sense" it shouldn't be taken into account for the lookup. One solution would be to differentiate this ideas syntactically:

mixin Mixin2 {

 requires method m1()      // this is not a method definition, just a constrain

 method m5()                    // should we still allow abstract methods definitions ?

}

None of this approaches have relative common ideas on other industrial languages, I guess. But it shouldn't matter if they are consistent. Scala for examples introduces also their not-so-natural ideas like the fact that a mixin can extends a class or a mixin, or "abstract override" keywords o typing "this". They are all cases to impose requirements over where the mixin can be combined.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/uqbar-project/wollok/issues/1002#issuecomment-250914054, or mute the thread https://github.com/notifications/unsubscribe-auth/AEa1OQW21g0lAV5ofMC3e4P5Zf1zrPJKks5qvmfygaJpZM4KLiDF .