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

Message ID 20181120115147.GA28760@delia
State New
Headers show
Series
  • [driver] Ensure --help=params lines end with period
Related show

Commit Message

Tom de Vries Nov. 20, 2018, 11:51 a.m.
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(-)

Comments

Jeff Law Nov. 20, 2018, 11:42 p.m. | #1
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
Mike Stump Nov. 26, 2018, 9:43 p.m. | #2
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

> +}
Tom de Vries Nov. 26, 2018, 10:22 p.m. | #3
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

>> +}

>

Patch

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
+}