RFR (S) : 8004967 - Default method cause java.lang.VerifyError: Illegal use of nonvirtual function call

[Bharadwaj Yadavalli]

Thanks again for the review, Vladimir.

On 1/3/2013 10:47 AM, Vladimir Kozlov wrote:
>     /* Legality of interface method modifiers for various Java versions
>        public:       Required in pre-JAVA_8_VERSION
>        abstract:     Required in pre-JAVA_8_VERSION
>
>
> In the original condition the combination ((is_synchronized || 
> is_strict) && is_abstract) was invalid for major_gte_8. Why you don't 
> have it?
>
The conditions for major_gte_8 in the currently checked in version are 
incomplete/incorrect. The primary intent of the proposed changes is to 
rectify that.

It now appears that this fix for the three failures of Lambda tests does 
not really address the the bug whose number is in the subject line. I am 
in the process of addressing them separately with separate code changes, 
bug reports and corresponding review requests.

> I would also move common invalid condition outside version checks:
>
>     if (is_native || is_protected || is_final) {
>       // Invalid in all class file versions.
>       is_illegal = true;
>     } else if (major_gte_8) {
>       // Class file version is JAVA_8_VERSION or later
>       if (is_public == is_private) {
>         // Only one of private and public should be true - XNOR
>         is_illegal = true;
>       }
>     } else if (!is_public || !is_abstract ||
>                is_private || is_static) {
>       // Invalid in pre-JAVA_8_VERSION versions
>       is_illegal = true;
>     } else if (major_gte_15) {
>       // Class file version in the interval [JAVA_1_5_VERSION, 
> JAVA_8_VERSION)
>       if (is_strict || is_synchronized) {
>         is_illegal = true;
>       }
>     } else {
>       // Class file version is pre-JAVA_1_5_VERSION
>       if (is_bridge || is_varargs || is_synthetic) {
>         is_illegal = true;
>       }
>     }
>

While moving common condition is definitely desirable, I believe keeping 
all invalid conditions together per version will allow easier 
maintainability and allow any future changes localized. It would be good 
not to end up with a convoluted combination of conditions like we 
currently have. It is not very clear as to which flags were checked and 
which were (inadvertently?) missed resulting in a potential mismatch 
between the JVM spec and the implementation.

Anyways, I'll have a new code review request once I update the validity 
checks for major_gte_8 to follow the JVM 8 spec changes.

Thanks,

Bharadwaj