RFR: 8243208: Clean up JVMFlag implementation [v2]

[Ioi Lam]

On Wed, 9 Sep 2020 00:55:07 GMT, David Holmes <dholmes at openjdk.org> wrote:

>> Ioi Lam has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   (a) coding style nits and comments, (b) updated HOWTO in globals.hpp, (c) added JVMFlag::check_all_flag_declarations(),
>>   (d) removed product_rw which is not used by any flags
>
> One query below and a couple of style nits. Otherwise LGTM!

Re: [Gerard Ziemski's comments on
hotspot-dev at openjdk.java.net](https://mail.openjdk.java.net/pipermail/hotspot-dev/2020-September/042868.html):

> (1) The API JVMFlagLimit::get_constraint(flag) says “constraint”, but it returns JVMFlagLimit* using different names
> for the same thing - one calls it “limit", the other uses “constraint”.

JVMFlagLimit is used for both constraints and ranges. I updated the comments:
// A JVMFlagLimit is created for each JVMFlag that has a range() and/or constraint() in its declaration in
// the globals_xxx.hpp file.
//
// To query the range information of a JVMFlag:
//     JVMFlagLimit::get_range(JVMFlag*)
//     JVMFlagLimit::get_range_at(int flag_enum)
// If the given flag doesn't have a range, NULL is returned.
//
// To query the constraint information of a JVMFlag:
//     JVMFlagLimit::get_constraint(JVMFlag*)
//     JVMFlagLimit::get_constraint_at(int flag_enum)
// If the given flag doesn't have a constraint, NULL is returned.

> (3) Isn’t there a better way to treat the special cases in "JVMFlagRangeList::print”, without explicitly checking for
> hardcoded function pointers? Or is it just not worth the effort, with only 2 such cases for now?

So far there are only 2 cases so I think the explicit checking may be enough. If we have more cases in the future we
can have a more general solution.

> (5) I'm not sure how others feel about it, but I think I personally would find it helpful for macros like
> “DEFINE_RANGE_CHECK” to see a comment with a concrete example of macro expansion for some flag, just to see what it
> works out to be.

I think this macro is fairly easy to read, so adding an expanded example may not have much benefit:

#define DEFINE_RANGE_CHECK(T)                                                            \
JVMFlag::Error JVMFlagRangeChecker::check_ ## T(T value, bool verbose) const {           \
  assert(exists(), "must be");                                                           \
  JVMFlagRange_ ## T range(_flag, _limit->as_ ## T()->min(), _limit->as_ ## T()->max()); \
  return range.check_ ## T(value, verbose);                                              \
}

In [JDK-8081833](https://bugs.openjdk.java.net/browse/JDK-8081833), I plan to use templates to replaces the nearly
identical code in jvmFlagRangeList.cpp/jvmFlagConstraintList.cpp, and these macros should go away.

(I fixed other minor nits found by Gerard.)

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

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