RFR: 8264707: HotSpot Style Guide should permit use of lambda [v2]

[Coleen Phillimore]

On Tue, 21 Sep 2021 22:05:09 GMT, Kim Barrett <kbarrett at openjdk.org> wrote:

>> Please review this proposal to permit the use of lambda expressions in
>> HotSpot code, with some restrictions and suggestions for good usage within
>> HotSpot code.  Lambda expressions were added in C++11, and provide a more
>> expressive syntax for local functions, with a number of use-cases where they
>> can improve readability by eliminating a lot of uninteresting boilerplate.
>> 
>> Some example uses are included, but are not part of the proposed change.
>> They will be removed from the PR before it is pushed.  (In particular, the
>> ScopeGuard utility uses move semantics, the use of which hasn't been
>> approved or even discussed.)  They are given to show some of the benefits
>> that might accrue from permitting the use of lambdas. In particular, they
>> highlight some of the code reduction that is possible. Some of these code
>> changes might be proposed in the future, using the normal PR process.
>> 
>> This is a modification of the Style Guide, so rough consensus among the
>> HotSpot Group members is required to make this change.  Only Group members
>> should vote for approval (via the github PR), though reasoned objections or
>> comments from anyone will be considered.  A decision on this proposal will
>> not be made before Wednesday 1-Sep-2021 at 12h00 UTC.
>> 
>> Since we're piggybacking on github PRs here, please use the PR review
>> process to approve (click on Review Changes > Approve), rather than sending
>> a "vote: yes" email reply that would be normal for a CFV.
>
> Kim Barrett has updated the pull request with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains five additional commits since the last revision:
> 
>  - Merge branch 'master' into permit_lambda
>  - terminology fix
>  - add scope guard and some example uses
>  - G1 SATB filter lambda
>  - new local functions section

I approve of adding lambdas as specified in the coding standard.

src/hotspot/share/compiler/compilerOracle.cpp line 787:

> 785: 
> 786:   char* original_line = os::strdup(line, mtInternal);
> 787:   auto g = make_guard([&] { os::free(original_line); });

I have to admit that this usage is very mysterious.  I know you're going to take it out but using lambdas for something that's really hard to parse doesn't seem like a win.  Does save lines of code though.

src/hotspot/share/gc/g1/g1SATBMarkQueueSet.cpp line 104:

> 102:     return !requires_marking(entry, _g1h) || _g1h->is_marked_next(cast_to_oop(entry));
> 103:   };
> 104:   apply_filter(requires_discard, queue);

This on the other hand, is a nice usage and saves a lot of boilerplate, and it's not like operator() was that easy to read in the first place.

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

Marked as reviewed by coleenp (Reviewer).

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