ceylon / ceylon-spec

DEPRECATED
Apache License 2.0
108 stars 34 forks source link

Add support for wildcard in the upper bound to improve Java interop #1386

Open pusolito opened 9 years ago

pusolito commented 9 years ago

Ceylon doesn't support wildcard in the upper bound which prevents interacts with Java like the following:

// Java class that operates on java.lang.Comparable

public class A<T extends Comparable> {}
// Ceylon code to work with above Java type

import java.lang {Comparable}
import java.time {LocalDateTime}

A<T> bar<T>(A<T> t) given T satisfies Comparable<T> => t;

void test() {
    bar<LocalDateTime>(A<LocalDateTime>()); //<============= Error [Type parameter T of declaration
    bar has argument LocalDateTime not assignable to upper bound Comparable<Object> of T]
}

From @gavinking:

Whoah:

public interface ChronoLocalDateTime<D extends ChronoLocalDate>  extends Temporal, >TemporalAdjuster, Comparable<ChronoLocalDateTime<?>>

OMG!

So it looks like what you're really wanting to write is this

A<T> bar<T>(A<T> t) given T satisfies Comparable<in T> => t; 

i.e. the wildcard in the supertype of ChronoLocalDateTime is obligating you to need a wildcard in the >upper bound, which is something Ceylon simply doesn't support, though it seems Java does. (I never >noticed that before.)

Open a feature request against ceylon-spec please, so that we can investigate if we can/should allow wildcards in upper bound type constraints.

gavinking commented 9 years ago

@RossTate thoughts? Are wildcards in upper bounds OK?

RossTate commented 9 years ago

Oops, I just saw this. As I mentioned in the thread, that's not necessary to solve the problem. You probably just have a glitch in importing the wildcard in the inheritance clause.

Here are the reductions that prove that the LocalDateTime case should be rejected and the ChronoLocalDateTime<LocalTime> cause should be accepted.

LocalDateTime case

  1. LocalDateTime <: Comparable<LocalDateTime>
  2. Comparable<ChronoLocalDateTime<?>> <: Comparable<LocalDateTime> (by inheritance)
  3. LocalDateTime <: ChronoLocalDateTime<?> (by contravariance)
  4. False - no such inheritance clause

ChronoLocalDateTime<LocalTime> case

  1. ChronoLocalDateTime<LocalTime> <: Comparable<ChronoLocalDateTime<LocalTime>>
  2. Comparable<ChronoLocalDateTime<?>> <: Comparable<ChronoLocalDateTime<LocalTime>> (by inheritance)
  3. ChronoLocalDateTime<LocalTime> <: ChronoLocalDateTime<?> (by contravariance)
  4. True by definition of ?
pusolito commented 9 years ago

@RossTate,

To clarify, this following code does not compile with Ceylon IDE 1.1.1.v20150721-1337-Final. The imports from Java are correct and verified.

Can you provide more specifics on how to get this working in the ChronoLocalDateTime at least? Or are you saying this doesn't work b/c of a separate bug within the import handling?

import java.lang {Comparable,Double}
import java.time {LocalDateTime, LocalDate}
import java.time.chrono {ChronoLocalDateTime}

A<T> bar<T>(A<T> t) given T satisfies Comparable<T> => t;

void test() {
    bar<Double>(A<Double>());
    bar<LocalDateTime>(A<LocalDateTime>());
    bar<ChronoLocalDateTime<LocalDate>>(A<ChronoLocalDateTime<LocalDate>>());
}

image

RossTate commented 9 years ago

Or are you saying this doesn't work b/c of a separate bug within the import handling?

Correct. And the feature proposed hear would not solve any of the problems. So I think this would be better classified as an issue for the compiler rather than for the spec. However, I'd first like @gavinking to chime in to confirm that my reasoning is sound.

Also, the LocalDateTime case will never work because it would be unsound to accept that. According to its definition, LocalDateTime is a subtype of Comparable<ChronoLocalDateTime<?>>, but that is not a subtype of Comparable<LocalDateTime> because Comparable is _contra_variant, not _co_variant (i.e. in not out).

FroMage commented 9 years ago

Actually, java.lang.Comparable is invariant since Java does not have declaration-site variance information.

gavinking commented 9 years ago

@RossTate

  1. ChronoLocalDateTime<LocalTime> <: Comparable<ChronoLocalDateTime<LocalTime>>

Where do you get that from? The declaration is:

    public interface ChronoLocalDateTime<D extends ChronoLocalDate>
            extends Temporal, TemporalAdjuster, Comparable<ChronoLocalDateTime<?>> { ... }

i.e. ChronoLocalDateTime<LocalTime> is a subtype of Comparable<ChronoLocalDateTime<?>> but the typechecker is complaining that it's not a subtype of Comparable<ChronoLocalDateTime<LocalTime>>

gavinking commented 9 years ago

@RossTate

  1. Comparable<ChronoLocalDateTime<?>> <: Comparable<ChronoLocalDateTime<LocalTime>> (by inheritance)

I'm also not seeing this one. Comparable is an invariant Java type, and ChronoLocalDateTime<?> isn't the same type as ChronoLocalDateTime<LocalTime>. So I don't think that step is correct at all. It would be correct for Ceylon's contravariant Comparable, but due to variance, not by inheritance.

  1. ChronoLocalDateTime<LocalTime> <: ChronoLocalDateTime<?> (by contravariance)

This is sound, but not quite for the stated reason: we treat ChronoLocalDateTime<?> as ChronoLocalDateTime<out Anything> in the model loader. So actually it's true by covariance ;-)

RossTate commented 9 years ago

You're misreading my proofs because I wrote them backwords. Read them as "in order to prove step 1, we must prove step 2, which then requires step 3, which then holds or doesn't hole due to step 4". The parentheticals indicate why the prior step reduces to that step.

Also, since it sounds like you're not inferring that Java's Comparable is contravariant, the signature of bar should be the following: A<T> bar<T>(A<T> t) given T satisfies Comparable<in T> => t;

Try that. If that works, then the problem was that @pusolito gave bar an unnecessarilly restrictive signature. If that doesn't work, then there's a glitch in the type importer or type checker.

gavinking commented 9 years ago

the signature of bar should be the following:

bar(A t) given T satisfies Comparable => t;

Right, precisely! That's exactly what I wrote in the issue description!

The problem is that right now we never allow a wildcard instantiation of a supertype, not even for type parameters (i.e. upper bounds). So I can't write that.

RossTate commented 9 years ago

Ohh, sorry, I didn't realize you were calling Comparable<in T> in Ceylon a wildcard instantiation. (It's technically just a use-site annotation.) So, yes, you should allow use-site annotations in bounds. My proofs already handle that.

gavinking commented 9 years ago

@RossTate cool, thanks.