Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Generic collection isn't being cast, causes compile error #352

Open
DSteve595 opened this issue Aug 7, 2018 · 14 comments
Open

Generic collection isn't being cast, causes compile error #352

DSteve595 opened this issue Aug 7, 2018 · 14 comments

Comments

@DSteve595
Copy link

With this simple class:

@Parcel
public class MyModel<T> {
    Set<Long> someLongs;
}

I'm getting the same error reported in #293. someLongs is indeed a Set<Long>, but the generated class can only see it as a Set<Object>. In 1.1.6, the generated code manually cast it to Set<Long>, which worked. In 1.1.7 through 1.1.11, the cast is gone, so compilation fails.

@johncarl81
Copy link
Owner

This is because you're asking Parceler to generate a Parcelable wrapper for an undefined generic type which resolves to Object. How are you using MyModel? Do you have a concrete implementation that extends it?

@DSteve595
Copy link
Author

DSteve595 commented Aug 7, 2018

Appreciate the quick response! I'm not using MyModel anywhere, I created it to try to boil down a model in a large project to just the part that was causing the issue. I still tested it in that project, but it isn't referenced or extended anywhere. Could the project's setup/dependencies possibly be an issue?

@johncarl81
Copy link
Owner

Ah, just reread the issue. Adding the generic <T> seem to cause the problem... something is happening here, ill have to dig in further.

@johncarl81
Copy link
Owner

johncarl81 commented Aug 7, 2018

This is interesting... Java issues a compiler error for defined generics in undefined genericised classes:

public class Tester {

    public static class Pass {
        Set<String> stuff;
    }

    public static class Fail<T> {
        Set<String> stuff;
    }
    
    public static class PassAgain extends Fail<Integer> {}

    public void run() {
        for (String stuff : new Pass().stuff) {}  // Good
        for (String stuff : new Fail().stuff) {}  // Compiler error
        for (String stuff : new Fail<>().stuff) {}  // Good
        for (String stuff : new Fail<Object>().stuff) {}  // Good
        for (String stuff : new PassAgain().stuff) {}  // Good
    }
}

... I guess because of type erasure?

@DSteve595
Copy link
Author

That's really weird. I wish I could help more, no idea why that would be happening. I'm sure there's some arcane historic JVM-related reason. Adding a cast should be a decent fix though, right? At worst, it'll be unnecessary (and probably compiled out).

@johncarl81
Copy link
Owner

We actually removed the cast a while ago because it was typically unnecessary. Here's another fun case:

public class Tester {

    public static class Fail<T> {
        Set<String> stuff;
    }
    
    public static class Referencing {
        Fail<Object> fail;
    }

    public void run() {
        for(String stuff : new Referencing().fail.stuff) {} // Good
    }

}

I wonder what a fix for this would look like. Honestly you shouldn't annotate a class with undefined generic parameters. Maybe we should issue an error in this case instead of attempting to generate code with errors?

@DSteve595
Copy link
Author

DSteve595 commented Aug 7, 2018

If it matters, in our codebase it's closer to public class MyModel<T extends MyInterface>.

@johncarl81
Copy link
Owner

In your codebase how is it used? Are there concrete implementations or is it referenced in a non-generic way as a field type (or something similar)?

@DSteve595
Copy link
Author

There are no subclasses, not totally sure if I'm following your examples, but essentially the generic type is an interface with an ID property, and we want those IDs in the parcel.

@johncarl81
Copy link
Owner

Sounds like it's the following case then:

@Parcel
public class SomeClass {
    MyModel<String> model;
}

@DSteve595
Copy link
Author

That might work, but I'm still not sure why the code generated for the original case is bad.

@johncarl81
Copy link
Owner

Sorry, the above case would not work, as long as MyModel<T> is annotated with @Parcel.

@DSteve595
Copy link
Author

No chance of the cast being reinstated?

@johncarl81
Copy link
Owner

I'm not eager to reinstate it. There is a deeper problem here that needs a more direct solution. I think the answer lies in understanding why generics behave in this way.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants