RFR 8130459(M): Add additional validation after heap creation

[sangheon.kim]

Hi Kim,

On 07/23/2015 10:20 AM, Kim Barrett wrote:
> On Jul 23, 2015, at 1:50 AM, sangheon.kim <sangheon.kim at oracle.com> wrote:
>> As I thought CommandLineFlags is a main class for command line flags, I wanted to concentrate all functionality there.
>> However separating check functionality to CommandLineFlagRangeList and CommandLineFlagConstraintList also seems reasonable.
>> So as you suggested I will move into CommandLineFlagRangeList and CommandLineFlagConstraintList.
>> And then we can eliminate some functions from CommandLineFlags class.
>>
>>> Eliminating the check_constraints_of_after_xxx functions would also
>>> involve moving the call to post_after_ergo_constraint_check, but I
>>> think that's misplaced inside
>>> CommandLineFlags::check_constraints_of_after_ergo.
>> I will change to call post_after_ergo_constraint_check directly since CommandLineFlags will not have constraint/range related functions.
>> But I don't think it is misplaced as CommandLineFlags managed all constraint/range check functions, adding post work function there seemed natural.
>> If we need 'AfterMemoryInit' post work, I was planing to add in CommandLineFlags::check_constraints_of_after_memory_init(). :)
>>
>>> Eliminating those check_constraints_of_after_xxx functions would have
>>> the additional benefit of removing any concerns about their names; I
>>> think another reviewer (David?) commented on them, and they strike me
>>> as rather strange.
>> I will change them.
>> But again I wanted to centralize range/constraint related functions at CommandLineFlags class. :-)
> I agree that CommandLineFlags is the main class for command line
> flags, but that doesn't mean all related functionality must be
> directly implemented by that class.
Yes, right.
What I meant was if possible I wanted to have all at CommandLineFlags.
But considering other things such as naming matters changed my mind to 
go your suggestion.
I also thought about this matter when I revised these parts. But 
couldn't find clear reason for separating.

>    Indeed, there are these related
> CommandLineFlag{Range,Contraint}[List] classes that provide some of
> the implementation of CommandLineFlags functionality.  And I expected
> to find the implementation of range and constraint checking with the
> corresponding classes, rather than with CommandLineFlags.
>
> Related to this, and specifically with the
> post_after_ergo_constraint_check function, I think Arguments is a
> layer that is built on top of CommandLineFlag and related classes
> (among others), and having something in the latter directly calling
> back up to Arguments is a layering breakage.
Yes, I agree regarding layering things.

>   I'd be fine with
> something like the following instead:
>
> Arguments::do_after_ergo (not wedded to this name)
>    // --- maybe check_ranges call goes here?
>    calls CommandLineFlagConstraintList::check_constraints with
>      CommandLineFlagConstraint::AfterErgo
>    calls post_after_ergo_constraint_check
>
> Or maybe that should just be at the end of apply_ergo?
Let me apply first suggestion.

Thanks,
Sangheon