[RFC][PATCH] Postpone print of --help=* option.

[RFC][PATCH] Postpone print of --help=* option.

[Martin Liška]

[-- Attachment #1: Type: text/plain, Size: 1078 bytes --]

Hi.

Last week I was curious which warnings are disabled by default on top
of -Wall and -Wextra. Thus I used --help=warning and noticed some discrepancy
in between documentation and output of the --help option.

I created PR89885 where I explained that OPT__help_ option handling happens
early. That's why LangEnabledBy are not reflected and similarly target overrides
don't take place.

I'm attaching diff for --help=warning for C++ and -Ofast.

Thoughts?

gcc/ChangeLog:

2019-04-01  Martin Liska  <mliska@suse.cz>

	* gcc.c (process_command): Add dummy file only
	if n_infiles == 0.
	* opts-global.c (decode_options): Pass lang_mask.
	* opts.c (print_help): New function.
	(finish_options): Print --help if help_option_argument
	is set.
	(common_handle_option): Factor out content of OPT__help_
	into print_help.
	* opts.h (finish_options): Add new argument.
---
 gcc/gcc.c         |   3 +-
 gcc/opts-global.c |   2 +-
 gcc/opts.c        | 267 ++++++++++++++++++++++++----------------------
 gcc/opts.h        |   3 +-
 4 files changed, 146 insertions(+), 129 deletions(-)



[-- Attachment #2: 0001-Postpone-print-of-help-option.patch --]
[-- Type: text/x-patch, Size: 9790 bytes --]

diff --git a/gcc/gcc.c b/gcc/gcc.c
index 4f57765b012..7ce1cae28a7 100644
--- a/gcc/gcc.c
+++ b/gcc/gcc.c
@@ -4751,7 +4751,8 @@ process_command (unsigned int decoded_options_count,
     }
 
   /* Ensure we only invoke each subprocess once.  */
-  if (print_subprocess_help || print_help_list || print_version)
+  if (n_infiles == 0
+      && (print_subprocess_help || print_help_list || print_version))
     {
       n_infiles = 0;
 
diff --git a/gcc/opts-global.c b/gcc/opts-global.c
index a5e9ef0237a..f110fe1026f 100644
--- a/gcc/opts-global.c
+++ b/gcc/opts-global.c
@@ -314,7 +314,7 @@ decode_options (struct gcc_options *opts, struct gcc_options *opts_set,
 			loc, lang_mask,
 			&handlers, dc);
 
-  finish_options (opts, opts_set, loc);
+  finish_options (opts, opts_set, loc, lang_mask);
 }
 
 /* Hold command-line options associated with stack limitation.  */
diff --git a/gcc/opts.c b/gcc/opts.c
index 02f6b4656e1..707e6754294 100644
--- a/gcc/opts.c
+++ b/gcc/opts.c
@@ -854,12 +854,18 @@ control_options_for_live_patching (struct gcc_options *opts,
     }
 }
 
+/* --help option argument if set.  */
+const char *help_option_argument = NULL;
+
+static void print_help (struct gcc_options *opts, unsigned int lang_mask);
+
+
 /* After all options at LOC have been read into OPTS and OPTS_SET,
    finalize settings of those options and diagnose incompatible
    combinations.  */
 void
 finish_options (struct gcc_options *opts, struct gcc_options *opts_set,
-		location_t loc)
+		location_t loc, unsigned int lang_mask)
 {
   enum unwind_info_type ui_except;
 
@@ -1223,6 +1229,10 @@ finish_options (struct gcc_options *opts, struct gcc_options *opts_set,
 					 opts->x_flag_live_patching,
 					 loc);
     }
+
+  /* Print --help=* if used.  */
+  if (help_option_argument != NULL)
+    print_help (opts, lang_mask);
 }
 
 #define LEFT_COLUMN	27
@@ -2052,6 +2062,135 @@ check_alignment_argument (location_t loc, const char *flag, const char *name)
   parse_and_check_align_values (flag, name, align_result, true, loc);
 }
 
+/* Print help when OPT__help_ is set.  */
+
+static void
+print_help (struct gcc_options *opts, unsigned int lang_mask)
+{
+  const char *a = help_option_argument;
+  unsigned int include_flags = 0;
+  /* Note - by default we include undocumented options when listing
+     specific classes.  If you only want to see documented options
+     then add ",^undocumented" to the --help= option.  E.g.:
+
+     --help=target,^undocumented  */
+  unsigned int exclude_flags = 0;
+
+  if (lang_mask == CL_DRIVER)
+    return;
+
+  /* Walk along the argument string, parsing each word in turn.
+     The format is:
+     arg = [^]{word}[,{arg}]
+     word = {optimizers|target|warnings|undocumented|
+     params|common|<language>}  */
+  while (*a != 0)
+    {
+      static const struct
+	{
+	  const char *string;
+	  unsigned int flag;
+	}
+      specifics[] =
+	{
+	    { "optimizers", CL_OPTIMIZATION },
+	    { "target", CL_TARGET },
+	    { "warnings", CL_WARNING },
+	    { "undocumented", CL_UNDOCUMENTED },
+	    { "params", CL_PARAMS },
+	    { "joined", CL_JOINED },
+	    { "separate", CL_SEPARATE },
+	    { "common", CL_COMMON },
+	    { NULL, 0 }
+	};
+      unsigned int *pflags;
+      const char *comma;
+      unsigned int lang_flag, specific_flag;
+      unsigned int len;
+      unsigned int i;
+
+      if (*a == '^')
+	{
+	  ++a;
+	  if (*a == '\0')
+	    {
+	      error ("missing argument to %qs", "--help=^");
+	      break;
+	    }
+	  pflags = &exclude_flags;
+	}
+      else
+	pflags = &include_flags;
+
+      comma = strchr (a, ',');
+      if (comma == NULL)
+	len = strlen (a);
+      else
+	len = comma - a;
+      if (len == 0)
+	{
+	  a = comma + 1;
+	  continue;
+	}
+
+      /* Check to see if the string matches an option class name.  */
+      for (i = 0, specific_flag = 0; specifics[i].string != NULL; i++)
+	if (strncasecmp (a, specifics[i].string, len) == 0)
+	  {
+	    specific_flag = specifics[i].flag;
+	    break;
+	  }
+
+      /* Check to see if the string matches a language name.
+	 Note - we rely upon the alpha-sorted nature of the entries in
+	 the lang_names array, specifically that shorter names appear
+	 before their longer variants.  (i.e. C before C++).  That way
+	 when we are attempting to match --help=c for example we will
+	 match with C first and not C++.  */
+      for (i = 0, lang_flag = 0; i < cl_lang_count; i++)
+	if (strncasecmp (a, lang_names[i], len) == 0)
+	  {
+	    lang_flag = 1U << i;
+	    break;
+	  }
+
+      if (specific_flag != 0)
+	{
+	  if (lang_flag == 0)
+	    *pflags |= specific_flag;
+	  else
+	    {
+	      /* The option's argument matches both the start of a
+		 language name and the start of an option class name.
+		 We have a special case for when the user has
+		 specified "--help=c", but otherwise we have to issue
+		 a warning.  */
+	      if (strncasecmp (a, "c", len) == 0)
+		*pflags |= lang_flag;
+	      else
+		warning (0,
+			 "--help argument %q.*s is ambiguous, "
+			 "please be more specific",
+			 len, a);
+	    }
+	}
+      else if (lang_flag != 0)
+	*pflags |= lang_flag;
+      else
+	warning (0,
+		 "unrecognized argument to --help= option: %q.*s",
+		 len, a);
+
+      if (comma == NULL)
+	break;
+      a = comma + 1;
+    }
+
+  if (include_flags)
+    print_specific_help (include_flags, exclude_flags, 0, opts,
+			 lang_mask);
+}
+
 /* Handle target- and language-independent options.  Return zero to
    generate an "unknown option" message.  Only options that need
    extra handling need to be listed here; if you simply want
@@ -2119,131 +2258,7 @@ common_handle_option (struct gcc_options *opts,
 
     case OPT__help_:
       {
-	const char *a = arg;
-	unsigned int include_flags = 0;
-	/* Note - by default we include undocumented options when listing
-	   specific classes.  If you only want to see documented options
-	   then add ",^undocumented" to the --help= option.  E.g.:
-
-	   --help=target,^undocumented  */
-	unsigned int exclude_flags = 0;
-
-	if (lang_mask == CL_DRIVER)
-	  break;
-
-	/* Walk along the argument string, parsing each word in turn.
-	   The format is:
-	   arg = [^]{word}[,{arg}]
-	   word = {optimizers|target|warnings|undocumented|
-		   params|common|<language>}  */
-	while (*a != 0)
-	  {
-	    static const struct
-	    {
-	      const char *string;
-	      unsigned int flag;
-	    }
-	    specifics[] =
-	    {
-	      { "optimizers", CL_OPTIMIZATION },
-	      { "target", CL_TARGET },
-	      { "warnings", CL_WARNING },
-	      { "undocumented", CL_UNDOCUMENTED },
-	      { "params", CL_PARAMS },
-	      { "joined", CL_JOINED },
-	      { "separate", CL_SEPARATE },
-	      { "common", CL_COMMON },
-	      { NULL, 0 }
-	    };
-	    unsigned int *pflags;
-	    const char *comma;
-	    unsigned int lang_flag, specific_flag;
-	    unsigned int len;
-	    unsigned int i;
-
-	    if (*a == '^')
-	      {
-		++a;
-		if (*a == '\0')
-		  {
-		    error_at (loc, "missing argument to %qs", "--help=^");
-		    break;
-		  }
-		pflags = &exclude_flags;
-	      }
-	    else
-	      pflags = &include_flags;
-
-	    comma = strchr (a, ',');
-	    if (comma == NULL)
-	      len = strlen (a);
-	    else
-	      len = comma - a;
-	    if (len == 0)
-	      {
-		a = comma + 1;
-		continue;
-	      }
-
-	    /* Check to see if the string matches an option class name.  */
-	    for (i = 0, specific_flag = 0; specifics[i].string != NULL; i++)
-	      if (strncasecmp (a, specifics[i].string, len) == 0)
-		{
-		  specific_flag = specifics[i].flag;
-		  break;
-		}
-
-	    /* Check to see if the string matches a language name.
-	       Note - we rely upon the alpha-sorted nature of the entries in
-	       the lang_names array, specifically that shorter names appear
-	       before their longer variants.  (i.e. C before C++).  That way
-	       when we are attempting to match --help=c for example we will
-	       match with C first and not C++.  */
-	    for (i = 0, lang_flag = 0; i < cl_lang_count; i++)
-	      if (strncasecmp (a, lang_names[i], len) == 0)
-		{
-		  lang_flag = 1U << i;
-		  break;
-		}
-
-	    if (specific_flag != 0)
-	      {
-		if (lang_flag == 0)
-		  *pflags |= specific_flag;
-		else
-		  {
-		    /* The option's argument matches both the start of a
-		       language name and the start of an option class name.
-		       We have a special case for when the user has
-		       specified "--help=c", but otherwise we have to issue
-		       a warning.  */
-		    if (strncasecmp (a, "c", len) == 0)
-		      *pflags |= lang_flag;
-		    else
-		      warning_at (loc, 0,
-				  "--help argument %q.*s is ambiguous, "
-				  "please be more specific",
-				  len, a);
-		  }
-	      }
-	    else if (lang_flag != 0)
-	      *pflags |= lang_flag;
-	    else
-	      warning_at (loc, 0,
-			  "unrecognized argument to --help= option: %q.*s",
-			  len, a);
-
-	    if (comma == NULL)
-	      break;
-	    a = comma + 1;
-	  }
-
-	if (include_flags)
-	  {
-	    target_option_override_hook ();
-	    print_specific_help (include_flags, exclude_flags, 0, opts,
-				 lang_mask);
-	  }
+	help_option_argument = arg;
 	opts->x_exit_after_options = true;
 	break;
       }
diff --git a/gcc/opts.h b/gcc/opts.h
index f14d9bcb896..6e99eaddbaf 100644
--- a/gcc/opts.h
+++ b/gcc/opts.h
@@ -418,7 +418,8 @@ extern bool target_handle_option (struct gcc_options *opts,
 				  void (*target_option_override_hook) (void));
 extern void finish_options (struct gcc_options *opts,
 			    struct gcc_options *opts_set,
-			    location_t loc);
+			    location_t loc,
+			    unsigned int lang_mask);
 extern void default_options_optimization (struct gcc_options *opts,
 					  struct gcc_options *opts_set,
 					  struct cl_decoded_option *decoded_options,


[-- Attachment #3: cpp-diff.txt --]
[-- Type: text/plain, Size: 5746 bytes --]

--- cc_old.txt	2019-04-01 13:33:30.802779271 +0200
+++ cc.txt	2019-04-01 13:39:38.014353425 +0200
@@ -12,7 +12,7 @@
   -Waggressive-loop-optimizations 	[enabled]
   -Waliasing                  		[disabled]
   -Walign-commons             		[enabled]
-  -Waligned-new=[none|global|all] 	none
+  -Waligned-new=[none|global|all] 	global
   -Wall                       		
   -Walloc-size-larger-than=   		-1
   -Walloc-zero                		[disabled]
@@ -35,9 +35,9 @@
   -Wbuiltin-macro-redefined   		[enabled]
   -Wc++-compat                		[disabled]
   -Wc++0x-compat              		
-  -Wc++11-compat              		[disabled]
-  -Wc++14-compat              		[disabled]
-  -Wc++17-compat              		[disabled]
+  -Wc++11-compat              		[enabled]
+  -Wc++14-compat              		[enabled]
+  -Wc++17-compat              		[enabled]
   -Wc++1z-compat              		
   -Wc-binding-type            		[disabled]
   -Wc11-c2x-compat            		[enabled]
@@ -50,12 +50,12 @@
   -Wcast-qual                 		[disabled]
   -Wcast-result               		[disabled]
   -Wcatch-value               		
-  -Wcatch-value=<0,3>         		0
+  -Wcatch-value=<0,3>         		1
   -Wchar-subscripts           		[enabled]
   -Wcharacter-truncation      		[disabled]
   -Wchkp                      		
   -Wclass-conversion          		[enabled]
-  -Wclass-memaccess           		[disabled]
+  -Wclass-memaccess           		[enabled]
   -Wclobbered                 		[enabled]
   -Wcomment                   		[enabled]
   -Wcomments                  		
@@ -71,9 +71,9 @@
   -Wdate-time                 		[disabled]
   -Wdeclaration-after-statement 	[enabled]
   -Wdelete-incomplete         		[enabled]
-  -Wdelete-non-virtual-dtor   		[disabled]
+  -Wdelete-non-virtual-dtor   		[enabled]
   -Wdeprecated                		[enabled]
-  -Wdeprecated-copy           		[disabled]
+  -Wdeprecated-copy           		[enabled]
   -Wdeprecated-copy-dtor      		[disabled]
   -Wdeprecated-declarations   		[enabled]
   -Wdesignated-init           		[enabled]
@@ -83,7 +83,7 @@
   -Wdiv-by-zero               		[enabled]
   -Wdo-subscript              		[disabled]
   -Wdouble-promotion          		[disabled]
-  -Wduplicate-decl-specifier  		[enabled]
+  -Wduplicate-decl-specifier  		[disabled]
   -Wduplicated-branches       		[disabled]
   -Wduplicated-cond           		[disabled]
   -Weffc++                    		[disabled]
@@ -117,7 +117,7 @@
   -Wif-not-aligned            		[enabled]
   -Wignored-attributes        		[enabled]
   -Wignored-qualifiers        		[enabled]
-  -Wimplicit                  		[enabled]
+  -Wimplicit                  		[disabled]
   -Wimplicit-fallthrough      		
   -Wimplicit-fallthrough=<0,5> 		3
   -Wimplicit-function-declaration 	[enabled]
@@ -127,7 +127,7 @@
   -Wincompatible-pointer-types 		[enabled]
   -Winherited-variadic-ctor   		[enabled]
   -Winit-list-lifetime        		[enabled]
-  -Winit-self                 		[disabled]
+  -Winit-self                 		[enabled]
   -Winline                    		[disabled]
   -Wint-conversion            		[enabled]
   -Wint-in-bool-context       		[enabled]
@@ -153,7 +153,7 @@
   -Wmemset-transposed-args    		[enabled]
   -Wmisleading-indentation    		[enabled]
   -Wmissing-attributes        		[enabled]
-  -Wmissing-braces            		[enabled]
+  -Wmissing-braces            		[disabled]
   -Wmissing-declarations      		[disabled]
   -Wmissing-field-initializers 		[enabled]
   -Wmissing-format-attribute  		
@@ -172,7 +172,7 @@
   -Wno-alloca-larger-than     		
   -Wno-vla-larger-than        		
   -Wnoexcept                  		[disabled]
-  -Wnoexcept-type             		[disabled]
+  -Wnoexcept-type             		[enabled]
   -Wnon-template-friend       		[enabled]
   -Wnon-virtual-dtor          		[disabled]
   -Wnonnull                   		[enabled]
@@ -196,13 +196,13 @@
   -Wpadded                    		[disabled]
   -Wparentheses               		[enabled]
   -Wpedantic                  		[disabled]
-  -Wpessimizing-move          		[disabled]
+  -Wpessimizing-move          		[enabled]
   -Wplacement-new             		
   -Wplacement-new=<0,2>       		-1
   -Wpmf-conversions           		[enabled]
-  -Wpointer-arith             		[disabled]
+  -Wpointer-arith             		[enabled]
   -Wpointer-compare           		[enabled]
-  -Wpointer-sign              		[enabled]
+  -Wpointer-sign              		[disabled]
   -Wpointer-to-int-cast       		[enabled]
   -Wpragmas                   		[enabled]
   -Wprio-ctor-dtor            		[enabled]
@@ -213,9 +213,9 @@
   -Wrealloc-lhs               		[disabled]
   -Wrealloc-lhs-all           		[disabled]
   -Wredundant-decls           		[disabled]
-  -Wredundant-move            		[disabled]
+  -Wredundant-move            		[enabled]
   -Wregister                  		[disabled]
-  -Wreorder                   		[disabled]
+  -Wreorder                   		[enabled]
   -Wrestrict                  		[enabled]
   -Wreturn-local-addr         		[enabled]
   -Wreturn-type               		[enabled]
@@ -295,7 +295,7 @@
   -Wunused-but-set-parameter  		[enabled]
   -Wunused-but-set-variable   		[enabled]
   -Wunused-const-variable     		
-  -Wunused-const-variable=<0,2> 		1
+  -Wunused-const-variable=<0,2> 		0
   -Wunused-dummy-argument     		[disabled]
   -Wunused-function           		[enabled]
   -Wunused-label              		[enabled]
@@ -315,7 +315,7 @@
   -Wvla                       		[enabled]
   -Wvla-larger-than=<number>  		-1
   -Wvolatile-register-var     		[enabled]
-  -Wwrite-strings             		[disabled]
+  -Wwrite-strings             		[enabled]
   -Wzero-as-null-pointer-constant 	[disabled]
   -Wzerotrip                  		[disabled]
   -frequire-return-statement  		[enabled]

[-- Attachment #4: Ofast-diff.txt --]
[-- Type: text/plain, Size: 1537 bytes --]

--- fast.txt	2019-04-01 13:51:17.520773198 +0200
+++ fast_old.txt	2019-04-01 13:53:03.170950231 +0200
@@ -5,13 +5,13 @@
   -Os                         		
   -faggressive-loop-optimizations 	[enabled]
   -falign-functions           		[enabled]
-  -falign-functions=          		
+  -falign-functions=          		16
   -falign-jumps               		[enabled]
-  -falign-jumps=              		
+  -falign-jumps=              		16:11:8
   -falign-labels              		[enabled]
-  -falign-labels=             		
+  -falign-labels=             		0:0:8
   -falign-loops               		[enabled]
-  -falign-loops=              		
+  -falign-loops=              		16:11:8
   -fassociative-math          		[enabled]
   -fassume-phsa               		[enabled]
   -fasynchronous-unwind-tables 		[enabled]
@@ -126,7 +126,7 @@
   -fprefetch-loop-arrays      		[enabled]
   -fprintf-return-value       		[enabled]
   -freciprocal-math           		[enabled]
-  -freg-struct-return         		[disabled]
+  -freg-struct-return         		[enabled]
   -frename-registers          		[enabled]
   -freorder-blocks            		[enabled]
   -freorder-blocks-algorithm=[simple|stc] 	stc
@@ -234,7 +234,7 @@
   -funroll-loops              		[disabled]
   -funsafe-math-optimizations 		[enabled]
   -funswitch-loops            		[enabled]
-  -funwind-tables             		[disabled]
+  -funwind-tables             		[enabled]
   -fvar-tracking              		[enabled]
   -fvar-tracking-assignments  		[enabled]
   -fvar-tracking-assignments-toggle 	[disabled]

Re: [RFC][PATCH] Postpone print of --help=* option.

[Jeff Law]

On 4/1/19 6:11 AM, Martin Liška wrote:
> Hi.
> 
> Last week I was curious which warnings are disabled by default on top
> of -Wall and -Wextra. Thus I used --help=warning and noticed some discrepancy
> in between documentation and output of the --help option.
> 
> I created PR89885 where I explained that OPT__help_ option handling happens
> early. That's why LangEnabledBy are not reflected and similarly target overrides
> don't take place.
> 
> I'm attaching diff for --help=warning for C++ and -Ofast.
> 
> Thoughts?
> 
> gcc/ChangeLog:
> 
> 2019-04-01  Martin Liska  <mliska@suse.cz>
> 
> 	* gcc.c (process_command): Add dummy file only
> 	if n_infiles == 0.
> 	* opts-global.c (decode_options): Pass lang_mask.
> 	* opts.c (print_help): New function.
> 	(finish_options): Print --help if help_option_argument
> 	is set.
> 	(common_handle_option): Factor out content of OPT__help_
> 	into print_help.
> 	* opts.h (finish_options): Add new argument.
> ---
>  gcc/gcc.c         |   3 +-
>  gcc/opts-global.c |   2 +-
>  gcc/opts.c        | 267 ++++++++++++++++++++++++----------------------
>  gcc/opts.h        |   3 +-
>  4 files changed, 146 insertions(+), 129 deletions(-)
> 
> 
> 
> 0001-Postpone-print-of-help-option.patch
> 
> diff --git a/gcc/gcc.c b/gcc/gcc.c
> index 4f57765b012..7ce1cae28a7 100644
> --- a/gcc/gcc.c
> +++ b/gcc/gcc.c
> @@ -4751,7 +4751,8 @@ process_command (unsigned int decoded_options_count,
>      }
>  
>    /* Ensure we only invoke each subprocess once.  */
> -  if (print_subprocess_help || print_help_list || print_version)
> +  if (n_infiles == 0
> +      && (print_subprocess_help || print_help_list || print_version))
>      {
>        n_infiles = 0;
The assignment to n_infiles is redundant after your change.  I suspect
the optimizers will catch this, so if you want to keep it for clarity
that's fine with me.

OK for the trunk.  Your call whether or not to remove the redundant
assignment.

jeff

Re: [RFC][PATCH] Postpone print of --help=* option.

[Martin Liška]

On 4/30/19 6:18 PM, Jeff Law wrote:
> On 4/1/19 6:11 AM, Martin Liška wrote:
>> Hi.
>>
>> Last week I was curious which warnings are disabled by default on top
>> of -Wall and -Wextra. Thus I used --help=warning and noticed some discrepancy
>> in between documentation and output of the --help option.
>>
>> I created PR89885 where I explained that OPT__help_ option handling happens
>> early. That's why LangEnabledBy are not reflected and similarly target overrides
>> don't take place.
>>
>> I'm attaching diff for --help=warning for C++ and -Ofast.
>>
>> Thoughts?
>>
>> gcc/ChangeLog:
>>
>> 2019-04-01  Martin Liska  <mliska@suse.cz>
>>
>> 	* gcc.c (process_command): Add dummy file only
>> 	if n_infiles == 0.
>> 	* opts-global.c (decode_options): Pass lang_mask.
>> 	* opts.c (print_help): New function.
>> 	(finish_options): Print --help if help_option_argument
>> 	is set.
>> 	(common_handle_option): Factor out content of OPT__help_
>> 	into print_help.
>> 	* opts.h (finish_options): Add new argument.
>> ---
>>  gcc/gcc.c         |   3 +-
>>  gcc/opts-global.c |   2 +-
>>  gcc/opts.c        | 267 ++++++++++++++++++++++++----------------------
>>  gcc/opts.h        |   3 +-
>>  4 files changed, 146 insertions(+), 129 deletions(-)
>>
>>
>>
>> 0001-Postpone-print-of-help-option.patch
>>
>> diff --git a/gcc/gcc.c b/gcc/gcc.c
>> index 4f57765b012..7ce1cae28a7 100644
>> --- a/gcc/gcc.c
>> +++ b/gcc/gcc.c
>> @@ -4751,7 +4751,8 @@ process_command (unsigned int decoded_options_count,
>>      }
>>  
>>    /* Ensure we only invoke each subprocess once.  */
>> -  if (print_subprocess_help || print_help_list || print_version)
>> +  if (n_infiles == 0
>> +      && (print_subprocess_help || print_help_list || print_version))
>>      {
>>        n_infiles = 0;
> The assignment to n_infiles is redundant after your change.  I suspect
> the optimizers will catch this, so if you want to keep it for clarity
> that's fine with me.
> 
> OK for the trunk.  Your call whether or not to remove the redundant
> assignment.

Thank you for review. I've removed the redundancy assignment before
commit.

Martin

> 
> jeff
> 

Re: [RFC][PATCH] Postpone print of --help=* option.

[Jakub Jelinek]

On Mon, Apr 01, 2019 at 02:11:17PM +0200, Martin Liška wrote:
> 2019-04-01  Martin Liska  <mliska@suse.cz>
> 
> 	* gcc.c (process_command): Add dummy file only
> 	if n_infiles == 0.
> 	* opts-global.c (decode_options): Pass lang_mask.
> 	* opts.c (print_help): New function.
> 	(finish_options): Print --help if help_option_argument
> 	is set.
> 	(common_handle_option): Factor out content of OPT__help_
> 	into print_help.
> 	* opts.h (finish_options): Add new argument.

As reported by Ulrich Drepper on IRC, this broke
--enable-as-accelerator-for=... build, tree-streamer-in.c calls
finish_options too and that caller has not been adjusted.

Any reason why you've called print_help from finish_options rather than
decode_options after it calls finish_options?

Or should tree-streamer-in.c pass CL_LTO as the lang_mask, or say CL_DRIVER
so that print_help does nothing in that case?

	Jakub

Re: [RFC][PATCH] Postpone print of --help=* option.

[Martin Liška]

On 5/2/19 12:30 PM, Jakub Jelinek wrote:
> On Mon, Apr 01, 2019 at 02:11:17PM +0200, Martin Liška wrote:
>> 2019-04-01  Martin Liska  <mliska@suse.cz>
>>
>> 	* gcc.c (process_command): Add dummy file only
>> 	if n_infiles == 0.
>> 	* opts-global.c (decode_options): Pass lang_mask.
>> 	* opts.c (print_help): New function.
>> 	(finish_options): Print --help if help_option_argument
>> 	is set.
>> 	(common_handle_option): Factor out content of OPT__help_
>> 	into print_help.
>> 	* opts.h (finish_options): Add new argument.
> 
> As reported by Ulrich Drepper on IRC, this broke
> --enable-as-accelerator-for=... build, tree-streamer-in.c calls
> finish_options too and that caller has not been adjusted.

You are right, it's guarded in #ifdef ACCEL_COMPILER
so I haven't seen the compilation error.

> 
> Any reason why you've called print_help from finish_options rather than
> decode_options after it calls finish_options?

Yes, I need that as I need option propagation, as described in:
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=89885#c0

> 
> Or should tree-streamer-in.c pass CL_LTO as the lang_mask, or say CL_DRIVER
> so that print_help does nothing in that case?

Yes, that would fix it. Can you please provide a patch that he can test?

Thanks,
Martin

> 
> 	Jakub
> 

Re: [RFC][PATCH] Postpone print of --help=* option.

[Jakub Jelinek]

On Thu, May 02, 2019 at 12:51:05PM +0200, Martin Liška wrote:
> You are right, it's guarded in #ifdef ACCEL_COMPILER
> so I haven't seen the compilation error.
> 
> > 
> > Any reason why you've called print_help from finish_options rather than
> > decode_options after it calls finish_options?
> 
> Yes, I need that as I need option propagation, as described in:
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=89885#c0

Well, that doesn't answer the question.
I was wondering why you couldn't:

2019-05-02  Jakub Jelinek  <jakub@redhat.com>

	* opts.h (finish_options): Remove lang_mask argument.
	(print_help, help_option_argument): Declare.
	* opts.c (print_help): Remove forward declaration, no longer static.
	(finish_options): Remove lang_mask argument, don't call print_help
	here.
	* opts-global.c (decode_options): Adjust finish_option caller, call
	print_help here.

--- gcc/opts.h.jj	2019-05-02 12:18:35.558051021 +0200
+++ gcc/opts.h	2019-05-02 13:00:53.488347939 +0200
@@ -418,8 +418,8 @@ extern bool target_handle_option (struct
 				  void (*target_option_override_hook) (void));
 extern void finish_options (struct gcc_options *opts,
 			    struct gcc_options *opts_set,
-			    location_t loc,
-			    unsigned int lang_mask);
+			    location_t loc);
+extern void print_help (struct gcc_options *opts, unsigned int lang_mask);
 extern void default_options_optimization (struct gcc_options *opts,
 					  struct gcc_options *opts_set,
 					  struct cl_decoded_option *decoded_options,
@@ -443,6 +443,8 @@ extern const struct sanitizer_opts_s
   bool can_recover;
 } sanitizer_opts[];
 
+extern const char *help_option_argument;
+
 extern void add_misspelling_candidates (auto_vec<char *> *candidates,
 					const struct cl_option *option,
 					const char *base_option);
--- gcc/opts.c.jj	2019-05-02 12:18:35.557051038 +0200
+++ gcc/opts.c	2019-05-02 13:00:34.058659611 +0200
@@ -858,15 +858,13 @@ control_options_for_live_patching (struc
 /* --help option argument if set.  */
 const char *help_option_argument = NULL;
 
-static void print_help (struct gcc_options *opts, unsigned int lang_mask);
-
 
 /* After all options at LOC have been read into OPTS and OPTS_SET,
    finalize settings of those options and diagnose incompatible
    combinations.  */
 void
 finish_options (struct gcc_options *opts, struct gcc_options *opts_set,
-		location_t loc, unsigned int lang_mask)
+		location_t loc)
 {
   enum unwind_info_type ui_except;
 
@@ -1230,10 +1228,6 @@ finish_options (struct gcc_options *opts
 					 opts->x_flag_live_patching,
 					 loc);
     }
-
-  /* Print --help=* if used.  */
-  if (help_option_argument != NULL)
-    print_help (opts, lang_mask);
 }
 
 #define LEFT_COLUMN	27
@@ -2066,7 +2060,7 @@ check_alignment_argument (location_t loc
 
 /* Print help when OPT__help_ is set.  */
 
-static void
+void
 print_help (struct gcc_options *opts, unsigned int lang_mask)
 {
   const char *a = help_option_argument;
--- gcc/opts-global.c.jj	2019-05-02 12:18:36.673033137 +0200
+++ gcc/opts-global.c	2019-05-02 13:01:25.024842046 +0200
@@ -314,7 +314,11 @@ decode_options (struct gcc_options *opts
 			loc, lang_mask,
 			&handlers, dc);
 
-  finish_options (opts, opts_set, loc, lang_mask);
+  finish_options (opts, opts_set, loc);
+
+  /* Print --help=* if used.  */
+  if (help_option_argument != NULL)
+    print_help (opts, lang_mask);
 }
 
 /* Hold command-line options associated with stack limitation.  */

	Jakub

Re: [RFC][PATCH] Postpone print of --help=* option.

[Martin Liška]

On 5/2/19 1:04 PM, Jakub Jelinek wrote:
> Well, that doesn't answer the question.
> I was wondering why you couldn't:

Ah sorry, you are right. The patch you suggested
is obviously nicer than what we have currently in trunk.

Feel free to install it.
Martin

[PATCH] Fix build with offloading (Re: [RFC][PATCH] Postpone print of --help=* option.)

[Jakub Jelinek]

Hi!

On Thu, May 02, 2019 at 01:04:07PM +0200, Jakub Jelinek wrote:
> Well, that doesn't answer the question.
> I was wondering why you couldn't:
> 
> 2019-05-02  Jakub Jelinek  <jakub@redhat.com>
> 
> 	* opts.h (finish_options): Remove lang_mask argument.
> 	(print_help, help_option_argument): Declare.
> 	* opts.c (print_help): Remove forward declaration, no longer static.
> 	(finish_options): Remove lang_mask argument, don't call print_help
> 	here.
> 	* opts-global.c (decode_options): Adjust finish_option caller, call
> 	print_help here.

On Thu, May 02, 2019 at 01:13:22PM +0200, Martin Liška wrote:
> On 5/2/19 1:04 PM, Jakub Jelinek wrote:
> > Well, that doesn't answer the question.
> > I was wondering why you couldn't:
> 
> Ah sorry, you are right. The patch you suggested
> is obviously nicer than what we have currently in trunk.

Bootstrapped/regtested successfully now on x86_64-linux and i686-linux, ok
for trunk?

	Jakub

Re: [PATCH] Fix build with offloading (Re: [RFC][PATCH] Postpone print of --help=* option.)

[Richard Biener]

[-- Attachment #1: Type: text/plain, Size: 1045 bytes --]

On Fri, 3 May 2019, Jakub Jelinek wrote:

> Hi!
> 
> On Thu, May 02, 2019 at 01:04:07PM +0200, Jakub Jelinek wrote:
> > Well, that doesn't answer the question.
> > I was wondering why you couldn't:
> > 
> > 2019-05-02  Jakub Jelinek  <jakub@redhat.com>
> > 
> > 	* opts.h (finish_options): Remove lang_mask argument.
> > 	(print_help, help_option_argument): Declare.
> > 	* opts.c (print_help): Remove forward declaration, no longer static.
> > 	(finish_options): Remove lang_mask argument, don't call print_help
> > 	here.
> > 	* opts-global.c (decode_options): Adjust finish_option caller, call
> > 	print_help here.
> 
> On Thu, May 02, 2019 at 01:13:22PM +0200, Martin Liška wrote:
> > On 5/2/19 1:04 PM, Jakub Jelinek wrote:
> > > Well, that doesn't answer the question.
> > > I was wondering why you couldn't:
> > 
> > Ah sorry, you are right. The patch you suggested
> > is obviously nicer than what we have currently in trunk.
> 
> Bootstrapped/regtested successfully now on x86_64-linux and i686-linux, ok
> for trunk?

OK.

Richard.

Re: [RFC][PATCH] Postpone print of --help=* option.

[Szabolcs Nagy]

On 01/04/2019 13:11, Martin Liška wrote:
> Hi.
> 
> Last week I was curious which warnings are disabled by default on top
> of -Wall and -Wextra. Thus I used --help=warning and noticed some discrepancy
> in between documentation and output of the --help option.
> 
> I created PR89885 where I explained that OPT__help_ option handling happens
> early. That's why LangEnabledBy are not reflected and similarly target overrides
> don't take place.
> 
> I'm attaching diff for --help=warning for C++ and -Ofast.
> 
> Thoughts?

since this change on arm-* and aarch64-* running RUNTESTFLAGS=help.exp i see

FAIL: compiler driver --help=params --help=optimizers option(s): "maximum number of" present in output
FAIL: compiler driver --help=params option(s): "[^.]$" absent from output: "e"

(indeed previously there were several 'max-*' params
in the output now there are none)

> 
> gcc/ChangeLog:
> 
> 2019-04-01  Martin Liska  <mliska@suse.cz>
> 
> 	* gcc.c (process_command): Add dummy file only
> 	if n_infiles == 0.
> 	* opts-global.c (decode_options): Pass lang_mask.
> 	* opts.c (print_help): New function.
> 	(finish_options): Print --help if help_option_argument
> 	is set.
> 	(common_handle_option): Factor out content of OPT__help_
> 	into print_help.
> 	* opts.h (finish_options): Add new argument.
> ---
>  gcc/gcc.c         |   3 +-
>  gcc/opts-global.c |   2 +-
>  gcc/opts.c        | 267 ++++++++++++++++++++++++----------------------
>  gcc/opts.h        |   3 +-
>  4 files changed, 146 insertions(+), 129 deletions(-)
> 
> 

Re: [RFC][PATCH] Postpone print of --help=* option.

[Martin Liška]

On 5/3/19 1:33 PM, Szabolcs Nagy wrote:
> On 01/04/2019 13:11, Martin Liška wrote:
>> Hi.
>>
>> Last week I was curious which warnings are disabled by default on top
>> of -Wall and -Wextra. Thus I used --help=warning and noticed some discrepancy
>> in between documentation and output of the --help option.
>>
>> I created PR89885 where I explained that OPT__help_ option handling happens
>> early. That's why LangEnabledBy are not reflected and similarly target overrides
>> don't take place.
>>
>> I'm attaching diff for --help=warning for C++ and -Ofast.
>>
>> Thoughts?
> 
> since this change on arm-* and aarch64-* running RUNTESTFLAGS=help.exp i see
> 
> FAIL: compiler driver --help=params --help=optimizers option(s): "maximum number of" present in output
> FAIL: compiler driver --help=params option(s): "[^.]$" absent from output: "e"
> 
> (indeed previously there were several 'max-*' params
> in the output now there are none)
> 
>>
>> gcc/ChangeLog:
>>
>> 2019-04-01  Martin Liska  <mliska@suse.cz>
>>
>> 	* gcc.c (process_command): Add dummy file only
>> 	if n_infiles == 0.
>> 	* opts-global.c (decode_options): Pass lang_mask.
>> 	* opts.c (print_help): New function.
>> 	(finish_options): Print --help if help_option_argument
>> 	is set.
>> 	(common_handle_option): Factor out content of OPT__help_
>> 	into print_help.
>> 	* opts.h (finish_options): Add new argument.
>> ---
>>  gcc/gcc.c         |   3 +-
>>  gcc/opts-global.c |   2 +-
>>  gcc/opts.c        | 267 ++++++++++++++++++++++++----------------------
>>  gcc/opts.h        |   3 +-
>>  4 files changed, 146 insertions(+), 129 deletions(-)
>>
>>
> 

Sure, there's a patch candidate:
https://gcc.gnu.org/ml/gcc-patches/2019-05/msg00114.html

and PR:
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=90315

Should be fixed soon.

Martin