Message ID | 20181120115147.GA28760@delia |
---|---|
State | New |
Headers | show |
Series | [driver] Ensure --help=params lines end with period | expand |
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 >> +} >
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 +}