[lworld] RFR: 8244227: [lworld] Explore an implementation where the reference projection and value projection types are backed by a single class symbol [v5]

[Srikanth Adayapalam]

On Thu, 8 Apr 2021 11:40:58 GMT, Srikanth Adayapalam <sadayapalam at openjdk.org> wrote:

>> In general, it looks good. I have three main comments:
>> * the refProjectionOrSelf trick is a good one, I believe
>> * consider using reference projection always when doing asSuper (e.g. adding that projection inside asSuper, directly)
>> * I believe the patch is replacing too many use sites with subtyping. There are several checks where we want to assert that T is an instance of C, where C can be AutoCloseable, Iterable, Serializable. It is not clear to me as to why we'd want these replaced with subtyping - in the sense that here we seem to be asking about a propoerty of the primitive class _declaration_ rather than the particular projection we happen to have on hand.
>> 
>> Note that, as we're exploring new territory, it is very possible that I have overlooked some underlying issue here - but thanks for taking the time to guide me through all the problems!
>
>> In general, it looks good. I have three main comments:
>> 
>> * the refProjectionOrSelf trick is a good one, I believe
>> * consider using reference projection always when doing asSuper (e.g. adding that projection inside asSuper, directly)
> 
> Will experiment with this and report back. This could minimize code changes. 
> 
>> * I believe the patch is replacing too many use sites with subtyping. There are several checks where we want to assert that T is an instance of C, where C can be AutoCloseable, Iterable, Serializable. It is not clear to me as to why we'd want these replaced with subtyping - in the sense that here we seem to be asking about a propoerty of the primitive class _declaration_ rather than the particular projection we happen to have on hand.
> 
> Possible. The rule of thumb I used for changing calls to asSuper into isSubtype was simply, whether (a) the call site is interested in the return value of asSuper as opoosed to just checking whether it is null or not and (b) whether generic class symbol could be passed in as the second argument to original asSuper call and if so retain it as asSuper itself. 
> 
> So it is likely more sites are changed than strictly necessary. The way I see it some of the overeaget changes to isSubtype result in equivalent semantics with no regression. Still it is worth minimizing ripples and for that reason I will attempt to restore these.
> 
>> 
>> Note that, as we're exploring new territory, it is very possible that I have overlooked some underlying issue here - but thanks for taking the time to guide me through all the problems!
> 
> I am grappling with similar challenges. I.e clearly characterizing which calls involve hazards of which kind. This involves reasoning about every site.

I pushed a commit accommodating two review comments:

1. asSuper calls at various places employed for the purpose of instanceof determination can be left alone.
2. Javadoc comment attached to asSuper is stronger than it ought to be and asserts something that is not quite true.

The third suggestion around asSuper internally coercing its argument into reference projection is something that will be taken up in a subsequent PR for JDK-8244712

@Maurizio, I'll proceed with integration if you give a quick glance through over the latest commit and approve. TIA

-------------

PR: https://git.openjdk.java.net/valhalla/pull/375