[driver] Ensure --help=params lines end with period

Hi,

this patch ensures that gcc --help=params lines end with a period by:
- fixing the help message of param HOT_BB_COUNT_FRACTION, and
- adding a test-case.

Build and tested on x86_64.

OK for trunk?

Thanks,
- Tom

[driver] Ensure --help=params lines end with period

2018-11-20  Tom de Vries  <tdevries@suse.de>

	PR c/79855
	* params.def (HOT_BB_COUNT_FRACTION): Terminate help message with
	period.

	* lib/options.exp (check_for_options_with_filter): New proc.
	* gcc.misc-tests/help.exp: Check that --help=params lines end with
	period.

---
 gcc/params.def                        |  2 +-
 gcc/testsuite/gcc.misc-tests/help.exp |  2 ++
 gcc/testsuite/lib/options.exp         | 34 ++++++++++++++++++++++++++++++----
 3 files changed, 33 insertions(+), 5 deletions(-)

On 11/20/18 4:51 AM, Tom de Vries wrote:
> Hi,
> 
> this patch ensures that gcc --help=params lines end with a period by:
> - fixing the help message of param HOT_BB_COUNT_FRACTION, and
> - adding a test-case.
> 
> Build and tested on x86_64.
> 
> OK for trunk?
> 
> Thanks,
> - Tom
> 
> [driver] Ensure --help=params lines end with period
> 
> 2018-11-20  Tom de Vries  <tdevries@suse.de>
> 
> 	PR c/79855
> 	* params.def (HOT_BB_COUNT_FRACTION): Terminate help message with
> 	period.
> 
> 	* lib/options.exp (check_for_options_with_filter): New proc.
> 	* gcc.misc-tests/help.exp: Check that --help=params lines end with
> 	period.
OK
jeff

On Nov 20, 2018, at 3:51 AM, Tom de Vries <tdevries@suse.de> wrote:
> 
> this patch ensures that gcc --help=params lines end with a period by:
> - fixing the help message of param HOT_BB_COUNT_FRACTION, and
> - adding a test-case.
> 
> Build and tested on x86_64.
> 
> OK for trunk?

So, normally we'd punt approval to a language maintainer or some other maintainer up the food chain.  :-)

The style of test is a hand cuff from which escape would be annoying, if people didn't want them all to end with a period.  Does someone else want to weigh in on good idea/bad idea here?

I'd tend to think this is a fine patch and would like to just approve it, but am happy to listen to a counter argument if people don't like it, also happy to let someone else approve/reject it if they want.

If no one feels strongly one way or other, I'll approve it.

> [driver] Ensure --help=params lines end with period
> 
> 2018-11-20  Tom de Vries  <tdevries@suse.de>
> 
> 	PR c/79855
> 	* params.def (HOT_BB_COUNT_FRACTION): Terminate help message with
> 	period.
> 
> 	* lib/options.exp (check_for_options_with_filter): New proc.
> 	* gcc.misc-tests/help.exp: Check that --help=params lines end with
> 	period.
> 
> ---
> gcc/params.def                        |  2 +-
> gcc/testsuite/gcc.misc-tests/help.exp |  2 ++
> gcc/testsuite/lib/options.exp         | 34 ++++++++++++++++++++++++++++++----
> 3 files changed, 33 insertions(+), 5 deletions(-)
> 
> diff --git a/gcc/params.def b/gcc/params.def
> index 2ae5a007530..11396a7f3af 100644
> --- a/gcc/params.def
> +++ b/gcc/params.def
> @@ -397,7 +397,7 @@ DEFPARAM(PARAM_SMS_LOOP_AVERAGE_COUNT_THRESHOLD,
> DEFPARAM(HOT_BB_COUNT_FRACTION,
> 	 "hot-bb-count-fraction",
> 	 "Select fraction of the maximal count of repetitions of basic block in program given basic "
> -	 "block needs to have to be considered hot (used in non-LTO mode)",
> +	 "block needs to have to be considered hot (used in non-LTO mode).",
> 	 10000, 0, 0)
> DEFPARAM(HOT_BB_COUNT_WS_PERMILLE,
> 	 "hot-bb-count-ws-permille",
> diff --git a/gcc/testsuite/gcc.misc-tests/help.exp b/gcc/testsuite/gcc.misc-tests/help.exp
> index f40cfabb41e..34ff9406e25 100644
> --- a/gcc/testsuite/gcc.misc-tests/help.exp
> +++ b/gcc/testsuite/gcc.misc-tests/help.exp
> @@ -63,6 +63,8 @@ check_for_options c "-v --help" "" {are likely to\n  -std} ""
> # Try various --help= classes and qualifiers.
> check_for_options c "--help=optimizers" "-O" "  -g  " ""
> check_for_options c "--help=params" "maximum number of" "-Wunsafe-loop-optimizations" ""
> +check_for_options_with_filter c "--help=params" \
> +    "^The --param option recognizes the following as parameters:$" "" {[^.]$} ""
> check_for_options c "--help=C" "-ansi" "-gnatO" ""
> check_for_options c {--help=C++} {-std=c\+\+} "-gnatO" ""
> check_for_options c "--help=common" "-dumpbase" "-gnatO" ""
> diff --git a/gcc/testsuite/lib/options.exp b/gcc/testsuite/lib/options.exp
> index 824d91276e4..60d85eea9d4 100644
> --- a/gcc/testsuite/lib/options.exp
> +++ b/gcc/testsuite/lib/options.exp
> @@ -26,11 +26,14 @@ if { [ishost "*-*-cygwin*"] } {
> }
> 
> # Run the LANGUAGE compiler with GCC_OPTIONS and inspect the compiler
> -# output to make sure that they match the newline-separated patterns
> -# in COMPILER_PATTERNS but not the patterns in COMPILER_NON_PATTERNS.
> -# In case of failure, xfail if XFAIL is nonempty.
> +# output excluding EXCLUDE lines to make sure that they match the
> +# newline-separated patterns in COMPILER_PATTERNS but not the patterns in
> +# COMPILER_NON_PATTERNS.  In case of failure, xfail if XFAIL is nonempty.
> 
> -proc check_for_options {language gcc_options compiler_patterns compiler_non_patterns expected_failure} {
> +proc check_for_options_with_filter { language gcc_options exclude \
> +					 compiler_patterns \
> +					 compiler_non_patterns \
> +					 expected_failure } {
>     set filename test-[pid]
>     set fd [open $filename.c w]
>     puts $fd "int main (void) { return 0; }"
> @@ -47,6 +50,21 @@ proc check_for_options {language gcc_options compiler_patterns compiler_non_patt
>     set gcc_output [gcc_target_compile $filename.c $filename.x executable $gcc_options]
>     remote_file build delete $filename.c $filename.x $filename.gcno
> 
> +    if { $exclude != "" } {
> +	set lines [split $gcc_output "\n"]
> +	set gcc_output ""
> +	foreach line $lines {
> +	    if {[regexp -line -- "$exclude" $line]} {
> +		continue
> +	    }
> +	    if { $gcc_output == "" } {
> +		set gcc_output "$line"
> +	    } else {
> +		set gcc_output "$gcc_output\n$line"
> +	    }
> +	}	
> +   }
> +    
>     # Verify that COMPILER_PATTERRNS appear in gcc output.
>     foreach pattern [split $compiler_patterns "\n"] {
> 	if {$pattern != ""} {
> @@ -79,3 +97,11 @@ proc check_for_options {language gcc_options compiler_patterns compiler_non_patt
> 	}
>     }
> }
> +
> +# As check_for_options_with_filter, but without the EXCLUDE parameter.
> +
> +proc check_for_options { language gcc_options compiler_patterns \
> +			     compiler_non_patterns expected_failure } {
> +    check_for_options_with_filter $language $gcc_options "" $compiler_patterns \
> +	$compiler_non_patterns $expected_failure
> +}

On 26-11-18 22:43, Mike Stump wrote:
> On Nov 20, 2018, at 3:51 AM, Tom de Vries <tdevries@suse.de> wrote:
>>
>> this patch ensures that gcc --help=params lines end with a period by:
>> - fixing the help message of param HOT_BB_COUNT_FRACTION, and
>> - adding a test-case.
>>
>> Build and tested on x86_64.
>>
>> OK for trunk?
> 
> So, normally we'd punt approval to a language maintainer or some other maintainer up the food chain.  :-)
> 

Hi Mike,

FYI the patch was already approved by Jeff, so I've committed it already.

> The style of test is a hand cuff from which escape would be annoying, if people didn't want them all to end with a period.  Does someone else want to weigh in on good idea/bad idea here?
> 
> I'd tend to think this is a fine patch 

Good to hear, thanks for the review.

- Tom

> and would like to just approve it, but am happy to listen to a counter argument if people don't like it, also happy to let someone else approve/reject it if they want.
> 
> If no one feels strongly one way or other, I'll approve it.
> 
>> [driver] Ensure --help=params lines end with period
>>
>> 2018-11-20  Tom de Vries  <tdevries@suse.de>
>>
>> 	PR c/79855
>> 	* params.def (HOT_BB_COUNT_FRACTION): Terminate help message with
>> 	period.
>>
>> 	* lib/options.exp (check_for_options_with_filter): New proc.
>> 	* gcc.misc-tests/help.exp: Check that --help=params lines end with
>> 	period.
>>
>> ---
>> gcc/params.def                        |  2 +-
>> gcc/testsuite/gcc.misc-tests/help.exp |  2 ++
>> gcc/testsuite/lib/options.exp         | 34 ++++++++++++++++++++++++++++++----
>> 3 files changed, 33 insertions(+), 5 deletions(-)
>>
>> diff --git a/gcc/params.def b/gcc/params.def
>> index 2ae5a007530..11396a7f3af 100644
>> --- a/gcc/params.def
>> +++ b/gcc/params.def
>> @@ -397,7 +397,7 @@ DEFPARAM(PARAM_SMS_LOOP_AVERAGE_COUNT_THRESHOLD,
>> DEFPARAM(HOT_BB_COUNT_FRACTION,
>> 	 "hot-bb-count-fraction",
>> 	 "Select fraction of the maximal count of repetitions of basic block in program given basic "
>> -	 "block needs to have to be considered hot (used in non-LTO mode)",
>> +	 "block needs to have to be considered hot (used in non-LTO mode).",
>> 	 10000, 0, 0)
>> DEFPARAM(HOT_BB_COUNT_WS_PERMILLE,
>> 	 "hot-bb-count-ws-permille",
>> diff --git a/gcc/testsuite/gcc.misc-tests/help.exp b/gcc/testsuite/gcc.misc-tests/help.exp
>> index f40cfabb41e..34ff9406e25 100644
>> --- a/gcc/testsuite/gcc.misc-tests/help.exp
>> +++ b/gcc/testsuite/gcc.misc-tests/help.exp
>> @@ -63,6 +63,8 @@ check_for_options c "-v --help" "" {are likely to\n  -std} ""
>> # Try various --help= classes and qualifiers.
>> check_for_options c "--help=optimizers" "-O" "  -g  " ""
>> check_for_options c "--help=params" "maximum number of" "-Wunsafe-loop-optimizations" ""
>> +check_for_options_with_filter c "--help=params" \
>> +    "^The --param option recognizes the following as parameters:$" "" {[^.]$} ""
>> check_for_options c "--help=C" "-ansi" "-gnatO" ""
>> check_for_options c {--help=C++} {-std=c\+\+} "-gnatO" ""
>> check_for_options c "--help=common" "-dumpbase" "-gnatO" ""
>> diff --git a/gcc/testsuite/lib/options.exp b/gcc/testsuite/lib/options.exp
>> index 824d91276e4..60d85eea9d4 100644
>> --- a/gcc/testsuite/lib/options.exp
>> +++ b/gcc/testsuite/lib/options.exp
>> @@ -26,11 +26,14 @@ if { [ishost "*-*-cygwin*"] } {
>> }
>>
>> # Run the LANGUAGE compiler with GCC_OPTIONS and inspect the compiler
>> -# output to make sure that they match the newline-separated patterns
>> -# in COMPILER_PATTERNS but not the patterns in COMPILER_NON_PATTERNS.
>> -# In case of failure, xfail if XFAIL is nonempty.
>> +# output excluding EXCLUDE lines to make sure that they match the
>> +# newline-separated patterns in COMPILER_PATTERNS but not the patterns in
>> +# COMPILER_NON_PATTERNS.  In case of failure, xfail if XFAIL is nonempty.
>>
>> -proc check_for_options {language gcc_options compiler_patterns compiler_non_patterns expected_failure} {
>> +proc check_for_options_with_filter { language gcc_options exclude \
>> +					 compiler_patterns \
>> +					 compiler_non_patterns \
>> +					 expected_failure } {
>>     set filename test-[pid]
>>     set fd [open $filename.c w]
>>     puts $fd "int main (void) { return 0; }"
>> @@ -47,6 +50,21 @@ proc check_for_options {language gcc_options compiler_patterns compiler_non_patt
>>     set gcc_output [gcc_target_compile $filename.c $filename.x executable $gcc_options]
>>     remote_file build delete $filename.c $filename.x $filename.gcno
>>
>> +    if { $exclude != "" } {
>> +	set lines [split $gcc_output "\n"]
>> +	set gcc_output ""
>> +	foreach line $lines {
>> +	    if {[regexp -line -- "$exclude" $line]} {
>> +		continue
>> +	    }
>> +	    if { $gcc_output == "" } {
>> +		set gcc_output "$line"
>> +	    } else {
>> +		set gcc_output "$gcc_output\n$line"
>> +	    }
>> +	}	
>> +   }
>> +    
>>     # Verify that COMPILER_PATTERRNS appear in gcc output.
>>     foreach pattern [split $compiler_patterns "\n"] {
>> 	if {$pattern != ""} {
>> @@ -79,3 +97,11 @@ proc check_for_options {language gcc_options compiler_patterns compiler_non_patt
>> 	}
>>     }
>> }
>> +
>> +# As check_for_options_with_filter, but without the EXCLUDE parameter.
>> +
>> +proc check_for_options { language gcc_options compiler_patterns \
>> +			     compiler_non_patterns expected_failure } {
>> +    check_for_options_with_filter $language $gcc_options "" $compiler_patterns \
>> +	$compiler_non_patterns $expected_failure
>> +}
>