Review request for OPENJDK6-40: OpenJDK6-b32 does not compile the testcase that Oracle JDK 6u45 compiles fine

[Nikolay Gorshkov]

Hi Andrew,

On 15.12.2014 18:40, Andrew Hughes wrote:
> The main thing that concerns me is the loss of the original author
> information, but I'm not sure if we have a solution for this as yet.
> I tend to go with the original author as the changeset author
> (hg commit --user <username>) but that also has the downside of
> not crediting the backporter.
>
> I guess both it and the original reviewer could be looked up via
> the changeset in 7, which is:
>
> 6938454: Unable to determine generic type in program that compiles under Java 6
> Summary: a redundant dubtyping check causes spurious inference failure
> Reviewed-by: jjg
>
> I think that's meant to be 'subtyping' ;)

As we discussed some time ago, for backports we could add a new
commit message header like "Authored-by: <original fix author>",
but it likely requires jcheck modification and updating official
rules.

> I hadn't realised this. Is there a reason the following hunks are absent?
>
> diff -r ed354a00f76b -r 36c4ec4525b4 src/share/classes/com/sun/tools/javac/comp/Attr.java
> --- a/src/share/classes/com/sun/tools/javac/comp/Attr.java	Tue Jul 27 11:52:11 2010 -0700
> +++ b/src/share/classes/com/sun/tools/javac/comp/Attr.java	Thu Jul 29 15:56:25 2010 +0100
> @@ -1694,8 +1694,22 @@
>               //if the type of the instance creation expression is an interface
>               //skip the method resolution step (JLS 15.12.2.7). The type to be
>               //inferred is of the kind <X1,X2, ... Xn>C<X1,X2, ... Xn>
> -            clazztype = new ForAll(clazztype.tsym.type.allparams(),
> -                    clazztype.tsym.type);
> +            clazztype = new ForAll(clazztype.tsym.type.allparams(), clazztype.tsym.type) {
> +                @Override
> +                public List<Type> getConstraints(TypeVar tv, ConstraintKind ck) {
> +                    switch (ck) {
> +                        case EXTENDS: return types.getBounds(tv);
> +                        default: return List.nil();
> +                    }
> +                }
> +                @Override
> +                public Type inst(List<Type> inferred, Types types) throws Infer.NoInstanceException {
> +                    // check that inferred bounds conform to their bounds
> +                    infer.checkWithinBounds(tvars,
> +                           types.subst(tvars, tvars, inferred), Warner.noWarnings);
> +                    return super.inst(inferred, types);
> +                }
> +            };
>           } else {
>               //if the type of the instance creation expression is a class type
>               //apply method resolution inference (JLS 15.12.2.7). The return type
>
> diff -r ed354a00f76b -r 36c4ec4525b4 src/share/classes/com/sun/tools/javac/comp/Infer.java
> --- a/src/share/classes/com/sun/tools/javac/comp/Infer.java	Tue Jul 27 11:52:11 2010 -0700
> +++ b/src/share/classes/com/sun/tools/javac/comp/Infer.java	Thu Jul 29 15:56:25 2010 +0100
> @@ -458,7 +457,7 @@
>
>       /** check that type parameters are within their bounds.
>        */
> -    private void checkWithinBounds(List<Type> tvars,
> +    void checkWithinBounds(List<Type> tvars,
>                                      List<Type> arguments,
>                                      Warner warn)
>           throws InvalidInstanceException {

These parts of the original fix were excluded because they are
related to diamond ("<>") operator support, which appeared in
Java 7 and is not a part of Java 6.

> Do the new tests added by the change pass?

Yes, I checked newly added testcases, and also all existing
javac jtreg tests (langtools/test/tools/javac) - all of them passed.

Thanks,
Nikolay

>
>> Thanks,
>>
>> Ivan
>>
>> 6938454: Unable to determine generic type in program that compiles under
>> Java 6
>> Reviewed-by: aph
>> Contributed-by: nikgor <nikolay at azulsystems.com>
>>
>>
>>
>> On 11/12/2014 22:01, Andrew Haley wrote:
>>> On 12/11/2014 06:59 PM, Nikolay Gorshkov wrote:
>>>> Me and my colleagues have a consensus that the patch is important to us -
>>>> we are responsible for introducing a regression into OpenJDK 6 (breaking
>>>> Apache Flume 1.5.0 compilation), and so we have to fix it. We have already
>>>> included this patch into our OpenJDK 6 -based products, and now we are
>>>> interested in upstreaming it to OpenJDK 6 itself to minimize the
>>>> difference
>>>> in codebase.
>>> Reluctantly, OK.  I am crossing my fingers and hoping that this
>>> is the last we'll see of this bug!  We shall see.
>>>
>>> Thanks for your patience,
>>> Andrew.
>>>
>>>
>>
>>
>
> Thanks,
>