RFR: 8276184: Exclude lambda proxy class from the CDS archive if its caller class is excluded [v5]

[Calvin Cheung]

On Wed, 10 Nov 2021 22:01:55 GMT, David Holmes <dholmes at openjdk.org> wrote:

> Hi Calvin,
> 
> I really don't like the existing style of this code in relation to warn_excluded and adding the additional "silent" parameter just makes this worse in my opinion. It is not clear why you only want to log a warning in some cases.
> 
> David

Hi David,

Thanks for taking a look.
It is to avoid logging the warning messages more than once.

thanks,
Calvin

> src/hotspot/share/classfile/systemDictionaryShared.cpp line 217:
> 
>> 215: 
>> 216:   if (!info->has_checked_exclusion()) {
>> 217:     if (check_for_exclusion_impl(k, false)) {
> 
> Please comment false argument.

I will add a comment.

> src/hotspot/share/classfile/systemDictionaryShared.cpp line 227:
> 
>> 225: 
>> 226: // Returns true so the caller can do:    return warn_excluded(".....");
>> 227: bool SystemDictionaryShared::warn_excluded(InstanceKlass* k, const char* reason, bool silent) {
> 
> This is an odd function. It is a bool but only ever returns true - so should be void. It is intended to log a warning but takes a "silent" parameter - seems the caller should simply skip the call in that case.

What if I leave the `warn_excluded()` function as before and make some change at the call sites. See below.

> src/hotspot/share/classfile/systemDictionaryShared.cpp line 274:
> 
>> 272: bool SystemDictionaryShared::check_for_exclusion_impl(InstanceKlass* k, bool silent) {
>> 273:   if (k->is_in_error_state()) {
>> 274:     return warn_excluded(k, "In error state", silent);
> 
> I do not like this coding style at all! All these "return warn_excluded" simply return true! And that "true" has nothing to do with the "warn" aspect.

The above return statement would become:
    `return silent ? true : warn_excluded(k, "In error state");`
What do you think?

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

PR: https://git.openjdk.java.net/jdk/pull/6205