ceylon / ceylon.language

DEPRECATED
Apache License 2.0
153 stars 57 forks source link

Allow metamodel invocation using a Map<String, Anything> #366

Closed FroMage closed 10 years ago

FroMage commented 10 years ago

For example a class could be instantiated with:

classDecl.instantiate(HashMap({"foo"->0,"bar"->true}));

and be equivalent to

myClass { foo = 0; bar = true; };
FroMage commented 10 years ago

As initially reported by @vietj at ceylon/ceylon-sdk#154

FroMage commented 10 years ago

I wonder if it's better to make the parameter type:

tombentley commented 10 years ago

I think Map makes most sense (or Correspondence if it can be reasonably implemented off that).

FroMage commented 10 years ago

It does from the implementor POV, but the invocation syntax will be classDecl.instantiate(HashMap{ "foo" -> 0, "bar" -> true }) which is longer and requires ceylon.collection.

FroMage commented 10 years ago

I don't see what using Correspondence would give us in this case, unless we have an implementation of Correspondence that isn't ceylon.collection::MutableMap?

tombentley commented 10 years ago

We don't have a Correspondence that isn't MutableMap yet, sure. But I can imagine a time when there's a Trie, for example which implements Correspondence but perhaps not Map. In general, we use the most general type we can, no?

FroMage commented 10 years ago

Well, actually it doesn't look like we can implement it at all because both Entry and Map require their Item to be Object and we need to be able to pass null arguments. I knew the whole restriction sucked from the start. I can, for some reason, use Correspondence<String,Anything>, but what use is it if I can't use neither Entry nor Map to fill it?

I'm very unsure how to proceed from there.

FroMage commented 10 years ago

Or I do something as silly as for ceylon.json and create a nullInstance singleton which represents a null you can put in a Map, and we go proliferating null alternate types everywhere…

tombentley commented 10 years ago

That does indeed suck. You could use a sequence of 2-tuples, which simply sucks in a different way.

Does anyone recall exactly why we disallowed null items in maps?

FroMage commented 10 years ago

Actually, I just looked at Correspondence for the first time properly and it's useless as a supertype. It's good for defining the behaviour of f[x] but not for abstraction: you can't iterate the keys/elements so there's no way to read a Correspondence if you don't know the keys. Well, admittedly we do know the keys from the function we're invoking, but still it feels a little unsatisfying as a contract. I'm not sure it's meant for more than defining the f[x] operator.

FroMage commented 10 years ago

Does anyone recall exactly why we disallowed null items in maps?

Same reason why we disallowed them in List and Set: because “it's heresy”? ;)

FroMage commented 10 years ago

So, I've pushed my changes:

@chochos: this now needs your love ;)

quintesse commented 10 years ago

I added the nullArgument singleton

Oh brrrr :(

(not criticizing. but it's obvious that what we feared will happen, everybody making their own "holes" sigh)

FroMage commented 10 years ago

It's only the second one. I do wonder how many eq we already have in the wild though ;)

chochos commented 10 years ago

this now needs your love

You keep using that word. I do not think it means what you think it means.

Seriously though, it's done. I had opened a separate issue in ceylon-js but I'm closing this one as well.

FroMage commented 10 years ago

Great!

gavinking commented 10 years ago

@FroMage This looks pretty ugly.

  1. You could just use {[String,Anything]*}.
  2. Why are we using strings here? We don't have metamodel refs to parameters yet?
gavinking commented 10 years ago

Note that this whole discussion is kinda wrong: the issue is not that a map/iterable/correspondence can't contain nulls; it clearly can! For example, HashMap { "foo"->1 } has a null for the key "bar". The issue is rather that @FroMage has two kinds of nulls here:

And he doesn't have any way to distinguish between these two kinds of meanings for "null". What he wants to be able to do is make this:

foo.namedApply(HashMap { "foo"->1, "bar"->null })

mean something different to this:

foo.namedApply(HashMap { "foo"->1 })

even though in both cases map["bar"] produces the same value null. I've argued repeatedly that a Map is a very bad way to represent such a situation. Fundamentally, if two maps have the same value for every given key, then they should be considered equal.

It seems to me that {[String,Anything]*} is the correct data structure to use in this case, though a {Argument*} might also be very reasonable.

gavinking commented 10 years ago

Note also, that I've argued a number of times that parameters of type T? should be limited to have the default argument null. This restriction would also have solved this issue.

gavinking commented 10 years ago

At least superficially, this really looks like a usecase for records, but what is proposed in ceylon/ceylon-spec#745 probably would not be a perfect solution for this usecase, since it doesn't really provide a good way to build a record incrementally, by adding attributes, which is, I imagine, required for the usecases that this feature addresses.

Still, you can imagine that if for every function foo(), we had an implied record type &foo, you could write:

&foo args = &foo { foo=1; bar=null; baz=""; };
foo.apply(args);

Note that there would be no need for a separate namedApply() method, since the record constructor &foo() does the job of unpacking the named args into a tuple. (According to ceylon/ceylon-spec#745, record types are basically just aliases for tuple types.)

That's rather elegant, but I don't see how you could build the record object incrementally. For that, I believe, you would need to have something more than is envisaged by ceylon/ceylon-spec#745. In fact, you would probably need a more conventional approach to record types where the type system provides the ability to primitively define a list of "labeled" values:

class { Integer foo; String baz; } args = value { value foo=1; value baz=""; };
class { Integer foo; String baz; String? bar; } moreArgs = args.with(value { value bar=null; });
foo.namedApply(args);
pthariensflame commented 10 years ago

I feel compelled to note that this is, fundamentally, why Maybe/Option is attractive over even a type-safe null: You can distinguish between None and Some(None), and Option<T> is a distinct type from Option<Option<T>>.

gavinking commented 10 years ago

@pthariensflame well, sure, but there's a whole lot of overhead associated with that, both for the programmer (who needs to explicitly wrap/unwrap the Maybe) and for the VM (which needs to maintain all these wrapper objects and gc them etc). I can imagine that for a VM with true value types, and arrays of value types, this might be acceptable. But it's not acceptable on a JVM or JavaScript VM where every one of these little wrapper objects gets allocated on the heap.

gavinking commented 10 years ago

@pthariensflame And even with Maybe I'm still left feeling queasy about saying

"foo"->null == "foo"->null

evaluates to true.

pthariensflame commented 10 years ago

@gavinking Well, you can always make Maybe a "nullable newtype", which would get you back exactly the runtime efficiency you have now, but your point about source-level boilerplate still stands. :)

FroMage commented 10 years ago

You could just use {[String,Anything]*}

Not if we want people to be able to use a Map<String,Anything>.

Why are we using strings here? We don't have metamodel refs to parameters yet?

Not sure anymore if we do or not, but we couldn't constrain them to belong to the current method if we did. Not statically. And we'd still have the use-case of frameworks who discover the methods and can't have static references of the right type, like we had to solve for invocation. Turns out so far all metamodel users have used the non-type-safe side.

As for null, what can I say? I still think it was a mistake to not allow it as value in collections, and I still think that "foo"->null == "foo"->null and null == null, like every other singleton value. I therefore also think it's too restrictive to force optional parameters to default to null. It proved a poor fit for JSON, and I still think lots of people will end up with their own null-like singleton to make up for it.

But remember that this also allows us to pass non-optional parameters of nullable type. Given foo(String? f) where f is required, if we want to pass it a null value explicitly we'd have to make a special case in the caller and remove every argument whose value is null, on the basis that the metamodel will substitute omitted arguments with null if the parameter is nullable, even if it is not optional. That seems like a far-fetched kludge to me.

But again, if you find that coherent with the rest of the state of null, I can make the change. Just be ready to see users work around these limitations.

gavinking commented 10 years ago

You could just use {[String,Anything]*}

Not if we want people to be able to use a Map<String,Anything>.

Well that doesn't really sound like an especially hard requirement.

I would much prefer to have a dedicated Arguments class than your nasty-ass nullArgument object.

Not sure anymore if we do or not, but we couldn't constrain them to belong to the current method if we did. Not statically.

Hrm, good point.

I still think that "foo"->null == "foo"->null and null == null, like every other singleton value.

But that's simply wrong, as I've pointed out many times. It leads to pathological bugs like:

if (customer1.ssn==customer2.ssn) {
    //it must be the same customer
}

Which is broken if ssn is nullable. The only case I can think of where it is correct that null==null evaluates to true is when you are doing a test with a literal null, i.e. of form x==null. But in Ceylon we never write that, we write exists x instead. In a test of form x==y it is almost never correct that the test is satisfied if x and y are both null. It's almost always a hidden bug.

I can make the change.

I think you should make the method accept {[String,Anything]*}, since that to me seems like the correct type for a list of named argument values. If you prefer, create a class called Arguments or even NamedArguments.

FroMage commented 10 years ago

Well that doesn't really sound like an especially hard requirement

It may be for frameworks. I guess they will almost always deal with Map in order to build the list. Converting it to something else than what Map is will be annoying. It is guessing, but that sounds like a safe assumption.

customer1.ssn==customer2.ssn

That's a bad example because it's not a problem with null values, but with how they chose a primary key. If the primary key cannot be null, then this problem does not exist.

We can create an Arguments class, but that only makes the calls more costly in terms of memory allocation. Well, that and that class would essentially be a Map which accepts null or not? And we have to hope that users will not want to use Map because they will be forced to convert from one to the other.

gavinking commented 10 years ago

Still sounds better and more convenient than your nullArgument to me.

FroMage commented 10 years ago

I'm not sure, it feels like the Arguments class will have to copy a lot of the behaviour of MutableMap to me.

gavinking commented 10 years ago

@pthariensflame Hrm, can you do Maybe as a newtype? That's not obvious to me. I'm probably forgetting something you already explained to me a while back...

gavinking commented 10 years ago

I'm not sure, it feels like the Arguments class will have to copy a lot of the behaviour of MutableMap to me.

Well, if you want it to be mutable. But is that appropriate for a language module class?

FroMage commented 10 years ago

How do you build one then?

gavinking commented 10 years ago

copyonwrite or builder.

FroMage commented 10 years ago

Can you give me a skeleton of API which allows you to build an Arguments instance one argument at a time (presumably but iterating the list of parameter declarations and figuring out which ones you have values for, or based on annotations, whatever), which allows for null values? I hate to guess ;)