jcoplien / trygve

The trygve language project - Building a DCI-centric language from the ground up
GNU General Public License v2.0
104 stars 15 forks source link

role player is bound but results in a null ref when used #64

Open runefs opened 8 years ago

runefs commented 8 years ago

When using the role directly in the constructor it results in a null ref. I personally think the use of the role should probably be illegal.

context Base {
    int c;
    role Parent {
        public void Blah(){
            System.out.println("Blah " + DoIt().toString());
        }
    }  requires {
        public int DoIt();
    }

    public int DoIt(){
        return c;
    }

    public Base(int ch, Base p){
        c = ch.clone;
        Parent = p;
        Parent.Blah();
    }

    public void Act(){ Parent.Blah(); }
}

{ 
    Base b = new Base(1,null);
    Base child = new Base(2,b);
    child.Act();
}
jcoplien commented 8 years ago

The code behaves as intended. I think you were maybe trying to describe another problem than what ensued from the behavior of this program, given the summary by which you describe the issue. The issue here is that the value null that you explicitly supplied as a Role-player is a valid Role-player. It is a special Role-player that can play any Role independent of that Role's requires declarations. Base.Base is called from the first line of the main script indicating null as the Role-player for Parent. When Parent.Blah() is enacted from the last line of the Base constructor, the Role semantics are sufficient to dispatch the call. You get in trouble only when trying to enact the DoIt() script of the null Role-player.

While this is a bit dubious, I think that to try either to warn against or prevent it in general would impose broader restrictions than we would tolerate. To disallow a null Role-player may cut off useful paths if and when we consider Role re-binding; and, in any case, that is exactly the binding used internally right now until overridden by the programmer. There is not yet a compile-time check for a Context constructor binding all Roles (NP-hard) so the null binding proves to be a useful sentinel in the mean time. I could cut things off earlier and castrate the Roll method dispatching mechanisms if they found there were a null Role-player, but that's either overly restrictive (it may be useful to have null Role-players) or upstaging the inevitable (these will be caught if any instance method messages are sent their way).

There's inadequate context to provide a compile-time warning and I want to limit (or maybe eliminate) run-time "warnings." Run-time anomalies should be fatal.

Right now, I'm inclined not to fix this. The code behaves as intended.

runefs commented 8 years ago

The original problem was not a problem, it was me being silly. However working from that code I found that you can't explicitly assign null to a role

public Base(int ch){
        c = ch.clone;
        Parent = null;
}

fails the type check of the RolePlayer when Parentis a role. Is this by design?

jcoplien commented 8 years ago

No, this is a bug. Null is a valid potential Role-player.

Degenerate case: Think of a special Context where all Role-players are null. One would use it just for the sake of dividing up a generic use case into perfect generic Roles.

Den 07/02/2016 kl. 09.18 skrev Rune Funch Søltoft notifications@github.com:

The original problem was not a problem, it was me being silly. However working from that code I found that you can't explicitly assign null to a role

public Base(int ch){ c = ch.clone; Parent = null; } fails the type check of the RolePlayer when Parentis a role. Is this by design?

— Reply to this email directly or view it on GitHub https://github.com/jcoplien/trygve/issues/64#issuecomment-180974542.

jcoplien commented 8 years ago

Den 07/02/2016 kl. 09.18 skrev Rune Funch Søltoft notifications@github.com:

The original problem was not a problem, it was me being silly. However working from that code I found that you can't explicitly assign null to a role

public Base(int ch){ c = ch.clone; Parent = null; } It must be more subtle than that. This compiles:

context con { role R { } Con() { R = null } }

I’ll explore further.

jcoplien commented 8 years ago

Hmmm. I see now what elicits the error message: a failure of Null to satisfy the signature of the requires block.

There’s something in me that takes comfort about being slapped on the hand about this, at least when I’m assigning directly to a Role. I’m inclined to leave it as is for the time being until we get a case that demonstrates its necessity.

jcoplien commented 8 years ago

I think this has been fixed.

runefs commented 8 years ago

I think it's incomprehensible why

context c {
   role b {}
   requires {
      void Foo();
   }
   c(){
      c rp = null;
      b = rp;
   }

   public void Foo(){}
}

should work and

context c {
   role b {}
   requires {
      void Foo();
   }
   c(){
      b = null;
   }

   public void Foo(){}
}

shouldn't. At best the former is simply a type cast of null to make sure the compiler knows the intended type of the object

jcoplien commented 8 years ago

The first shouldn’t work but, in general, to detect these cases is NP-hard.

Any suggestions on how to redress this? I could put full data flow analysis into the compiler, but I fear that will delay things by a couple of years, and it’s probably a rewrite… Even then, it wouldn’t catch all cases so ends only as a heuristic.

I’m inclined to mark this as “Won’t fix.” Arguments?

Den 09/02/2016 kl. 10.00 skrev Rune Funch Søltoft notifications@github.com:

I think it's incomprehensible why

context c { role b {} requires { void Foo(); } c(){ c rp = null; b = rp; }

public void Foo(){} } should work and

context c { role b {} requires { void Foo(); } c(){ b = null; }

public void Foo(){} } shouldn't. At best the former is simply a type cast of null to make sure the compiler knows the intended type of the object

— Reply to this email directly or view it on GitHub https://github.com/jcoplien/trygve/issues/64#issuecomment-181767900.

runefs commented 8 years ago

I think there's two potentially solutions. One allowing null to be bound as a roleplayer implicitly and explicitly and one disallowing null as a roleplayer altogether. The current is hard to express. You are allowed to bind null as the player, unless you do this explicitly and the role has a requires clause

2016-02-09 10:46 GMT+01:00 James O. Coplien notifications@github.com:

The first shouldn’t work but, in general, to detect these cases is NP-hard.

Any suggestions on how to redress this? I could put full data flow analysis into the compiler, but I fear that will delay things by a couple of years, and it’s probably a rewrite… Even then, it wouldn’t catch all cases so ends only as a heuristic.

I’m inclined to mark this as “Won’t fix.” Arguments?

Den 09/02/2016 kl. 10.00 skrev Rune Funch Søltoft < notifications@github.com>:

I think it's incomprehensible why

context c { role b {} requires { void Foo(); } c(){ c rp = null; b = rp; }

public void Foo(){} } should work and

context c { role b {} requires { void Foo(); } c(){ b = null; }

public void Foo(){} } shouldn't. At best the former is simply a type cast of null to make sure the compiler knows the intended type of the object

— Reply to this email directly or view it on GitHub < https://github.com/jcoplien/trygve/issues/64#issuecomment-181767900>.

— Reply to this email directly or view it on GitHub https://github.com/jcoplien/trygve/issues/64#issuecomment-181785617.

jcoplien commented 8 years ago

FWIW, I noticed that Trygve's front-loading example heavily uses nil as a sentinel value in Role-bindings and that he frequently re-binds Roles during execution... So there's a strong precedent for a null Role binding.

Also, maybe it's enough with the new type conversion rules (as of 1.5.14) that your above this-should-work example indeed does work. There's a lot more intelligence in the matching of the requires clause with both the Role-players and with the methods that are clients of the Role / object interface.

See if this is good enough to allay your concern. If not, I'm going to have to introduce a new, special variant of null to designate unbound Roles. The semantics are a bit messy but I believe they can be worked out.

jcoplien commented 2 years ago

As conjectured in the previous comment, this code which you suggested "should work" indeed does work:

context c { role b {} requires { void Foo(); } public c(){ c rp = null; b = rp; } public void Foo(){} }

(new c())

So if your last comment above is your "last work" on this, I think we are there.