On 06/20/2018 02:38 AM, Martin Liška wrote:
> On 06/12/2018 09:41 PM, Jeff Law wrote:
>> On 06/04/2018 07:32 AM, marxin wrote:
>>> gcc/ChangeLog:
>>>
>>> 2018-06-07  Martin Liska  <mli...@suse.cz>
>>>
>>>     * tree-switch-conversion.c (MAX_CASE_BIT_TESTS): Remove.
>>>     (hoist_edge_and_branch_if_true): Likewise.
>>>     (expand_switch_using_bit_tests_p): Likewise.
>>>     (struct case_bit_test): Likewise.
>>>     (case_bit_test_cmp): Likewise.
>>>     (emit_case_bit_tests): Likewise.
>>>     (switch_conversion::switch_conversion): New class.
>>>     (struct switch_conv_info): Remove old struct.
>>>     (collect_switch_conv_info): More to ...
>>>     (switch_conversion::collect): ... this.
>>>     (check_range): Likewise.
>>>     (switch_conversion::check_range): Likewise.
>>>     (check_all_empty_except_final): Likewise.
>>>     (switch_conversion::check_all_empty_except_final): Likewise.
>>>     (check_final_bb): Likewise.
>>>     (switch_conversion::check_final_bb): Likewise.
>>>     (create_temp_arrays): Likewise.
>>>     (switch_conversion::create_temp_arrays): Likewise.
>>>     (free_temp_arrays): Likewise.
>>>     (gather_default_values): Likewise.
>>>     (switch_conversion::gather_default_values): Likewise.
>>>     (build_constructors): Likewise.
>>>     (switch_conversion::build_constructors): Likewise.
>>>     (constructor_contains_same_values_p): Likewise.
>>>     (switch_conversion::contains_same_values_p): Likewise.
>>>     (array_value_type): Likewise.
>>>     (switch_conversion::array_value_type): Likewise.
>>>     (build_one_array): Likewise.
>>>     (switch_conversion::build_one_array): Likewise.
>>>     (build_arrays): Likewise.
>>>     (switch_conversion::build_arrays): Likewise.
>>>     (gen_def_assigns): Likewise.
>>>     (switch_conversion::gen_def_assigns): Likewise.
>>>     (prune_bbs): Likewise.
>>>     (switch_conversion::prune_bbs): Likewise.
>>>     (fix_phi_nodes): Likewise.
>>>     (switch_conversion::fix_phi_nodes): Likewise.
>>>     (gen_inbound_check): Likewise.
>>>     (switch_conversion::gen_inbound_check): Likewise.
>>>     (process_switch): Use the newly created class.
>>>     (switch_conversion::expand): New.
>>>     (switch_conversion::~switch_conversion): New.
>>>     * tree-switch-conversion.h: New file.
>> So generally this looks good.
>>
>> I do note that a lot of the function comments are removed from the
>> implementation side and moved into the header file.
> 
> Hi.
> 
> Thanks for review.
> 
>>
>> I personally prefer that approach, particularly for publicly visible
>> members.  However I think our coding conventions still indicate the
>> implementation side should have the function comment.  So let's keep
>> those in place, with obvious changes since many of the arguments are now
>> class data.
> 
> I'm going to change it in order to follow our coding style.
> Question is whether we can change that for future? Note that
> there are quite long function comments in tree-switch-conversion.h.
I'll support a change to the conventions.   In a perfect world the .h
file would provide an interface and the comments in there would be
sufficient for anyone to use the interface.  Any comments in the .c
would cover implementation issues.

I don't know if we'll get to that point, but that's the mental model in
my head.

jeff

Reply via email to