Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP

Loading…

Array underlying backing array uses java.lang.Object[] for optional types #402

Closed
FroMage opened this Issue · 10 comments

3 participants

@FroMage
Owner

Which means the following doesn't pass anymore:

test void compilerBug640(){
    value x = Array<Accept?> { Accept(), Accept(), Accept() };
    Accept().accepts(javaObjectArray(x));
}
public class Accept {
    public void longs(long[] args) { }
    public void strings(String[] args) { }
    public void accepts(Accept[] args) { }
}
@gavinking gavinking was assigned
@gavinking
Owner

This makes me realize that the mechanism we use to determine if we have an array of a "primitive" type is actually quite broken. If I have an Array<Integer|Integer>, it will not realize that this is really an Array<Integer>. So we're going to need to fully materialize the reified type when instantiating arrays.

@gavinking
Owner

So where do I get a RuntimeModuleManager from, in order to do that?

@gavinking
Owner

So we're going to need to fully materialize the reified type when instantiating arrays.

Looks like I've figured out a solution that doesn't require this.

@gavinking gavinking referenced this issue from a commit
@gavinking gavinking Solution to #402 903a058
@gavinking
Owner

We need a lot more tests for this stuff!

@gavinking
Owner

We're also going to need special cases for Identifiable, Basic, and Anything.

@gavinking
Owner

So there was a whole wide family of bugs here, of which this was just one. I think I've got most of the cases handled now, though I would not be remotely surprised if there's some I've missed.

Basically, we need waaay more tests here, since the tests that exist don't begin to cover all the special cases that exist:

  • union and intersection types
  • optional types vs definite types
  • erased types
  • Java arrays
@gavinking
Owner

Closing.

@gavinking gavinking closed this
@tombentley
Collaborator

Why've you closed this when it looks like we still need more tests?

@gavinking
Owner

@tombentley I don't know what tests are required.

@tombentley
Collaborator

Well, you wrote a bulleted list of types we ought to be testing, let's at least make sure we have tests for those.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.