public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* options: Fix 'enabledby_negargs' typo in 'LangEnabledBy' option property diagnostics (was: r193303 - in /trunk/gcc: ChangeLog opt-function...)
       [not found] <20121107181102.28015.qmail@sourceware.org>
@ 2022-03-29  9:21 ` Thomas Schwinge
  2022-03-29  9:28 ` options: Improve " Thomas Schwinge
  1 sibling, 0 replies; 3+ messages in thread
From: Thomas Schwinge @ 2022-03-29  9:21 UTC (permalink / raw)
  To: manu, gcc-patches; +Cc: Joseph Myers

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

Hi!

On 2012-11-07T18:11:02+0000, manu@gcc.gnu.org wrote:
> Author: manu
> Date: Wed Nov  7 18:11:01 2012
> New Revision: 193303
>
> URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=193303
> Log:
> 2012-11-07  Manuel López-Ibáñez  <manu@gcc.gnu.org>
>
>       * optc-gen.awk: Factor code out to...
>       * opt-functions.awk (lang_enabled_by): ... this new function.
>
> Modified:
>     trunk/gcc/ChangeLog
>     trunk/gcc/opt-functions.awk
>     trunk/gcc/optc-gen.awk

In improving the diagnostic, this has introduced a 'enabledby_negargs'
typo:

| --- gcc/optc-gen.awk
| +++ gcc/optc-gen.awk
| [...]
| -     enabledby_negarg = nth_arg(3, enabledby_arg)
| [...]
| -            if (enabledby_posarg != "" && enabledby_negarg != "") {
| -                with_args = "," enabledby_posarg "," enabledby_negarg
| -            } else if (enabledby_posarg == "" && enabledby_negarg == "") {
| -                with_args = ""
| -            } else {
| -                print "#error LangEnabledBy with three arguments, it should have either 2 or 4"
| -            }
| [...]
| +        lang_enabled_by(enabledby_langs, enabledby_name, enabledby_posarg, enabledby_negarg);
| [...]

| --- gcc/opt-functions.awk
| +++ gcc/opt-functions.awk
| [...]
| +function lang_enabled_by(enabledby_langs, enabledby_name, enabledby_posarg, enabledby_negarg)
| +{
| [...]
| +        if (enabledby_posarg != "" && enabledby_negarg != "") {
| +            with_args = "," enabledby_posarg "," enabledby_negarg
| +        } else if (enabledby_posarg == "" && enabledby_negarg == "") {
| +            with_args = ""
| +        } else {
| +            print "#error LangEnabledBy("enabledby_langs","enabledby_name", " \
| +                enabledby_posarg", " enabledby_negargs                  \
| +                ") with three arguments, it should have either 2 or 4"
| [...]

As obvious, pushed to master branch
commit 2788d42bdc66988449d40bf638f203a7cffc6cd9 "options: Fix
'enabledby_negargs' typo in 'LangEnabledBy' option property diagnostics",
see attached.


Grüße
 Thomas


-----------------
Siemens Electronic Design Automation GmbH; Anschrift: Arnulfstraße 201, 80634 München; Gesellschaft mit beschränkter Haftung; Geschäftsführer: Thomas Heurung, Frank Thürauf; Sitz der Gesellschaft: München; Registergericht München, HRB 106955

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-options-Fix-enabledby_negargs-typo-in-LangEnabledBy-.patch --]
[-- Type: text/x-diff, Size: 1799 bytes --]

From 2788d42bdc66988449d40bf638f203a7cffc6cd9 Mon Sep 17 00:00:00 2001
From: Thomas Schwinge <thomas@codesourcery.com>
Date: Mon, 28 Mar 2022 10:25:16 +0200
Subject: [PATCH] options: Fix 'enabledby_negargs' typo in 'LangEnabledBy'
 option property diagnostics

Originally introduced almost ten years ago in
r193303/commit 0829c7f7c5210cd1581042115cfe95c97283f44c
"optc-gen.awk: Factor code out to...".

	gcc/
	* opt-functions.awk (lang_enabled_by): Fix 'enabledby_negargs'
	typo.
---
 gcc/opt-functions.awk | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/gcc/opt-functions.awk b/gcc/opt-functions.awk
index 5b0bc6634f5..9eccf9b0409 100644
--- a/gcc/opt-functions.awk
+++ b/gcc/opt-functions.awk
@@ -388,7 +388,7 @@ function lang_enabled_by(enabledby_langs, enabledby_name, enabledby_posarg, enab
         with_args = ""
     } else {
         print "#error " opts[i] " LangEnabledBy("enabledby_langs","enabledby_name", " \
-            enabledby_posarg", " enabledby_negargs                  \
+            enabledby_posarg", " enabledby_negarg \
             ") with three arguments, it should have either 2 or 4"
     }
 
@@ -397,7 +397,7 @@ function lang_enabled_by(enabledby_langs, enabledby_name, enabledby_posarg, enab
         enabledby_index = opt_numbers[enabledby_array[k]];
         if (enabledby_index == "") {
              print "#error " opts[i] " LangEnabledBy("enabledby_langs","enabledby_name", " \
-                 enabledby_posarg", " enabledby_negargs"), unknown option '" enabledby_name "'"
+                 enabledby_posarg", " enabledby_negarg"), unknown option '" enabledby_name "'"
         } else {
             for (j = 1; j <= n_enabledby_arg_langs; j++) {
                  lang_name = lang_sanitized_name(enabledby_arg_langs[j]);
-- 
2.35.1


^ permalink raw reply	[flat|nested] 3+ messages in thread

* options: Improve 'LangEnabledBy' option property diagnostics (was: r193303 - in /trunk/gcc: ChangeLog opt-function...)
       [not found] <20121107181102.28015.qmail@sourceware.org>
  2022-03-29  9:21 ` options: Fix 'enabledby_negargs' typo in 'LangEnabledBy' option property diagnostics (was: r193303 - in /trunk/gcc: ChangeLog opt-function...) Thomas Schwinge
@ 2022-03-29  9:28 ` Thomas Schwinge
  2022-03-29 19:37   ` Joseph Myers
  1 sibling, 1 reply; 3+ messages in thread
From: Thomas Schwinge @ 2022-03-29  9:28 UTC (permalink / raw)
  To: manu, gcc-patches, Joseph Myers

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

Hi!

Once the three 'LangEnabledBy' option property patches that I've just
posted are accepted, we may proceed here:

On 2012-11-07T18:11:02+0000, manu@gcc.gnu.org wrote:
> Author: manu
> Date: Wed Nov  7 18:11:01 2012
> New Revision: 193303
>
> URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=193303
> Log:
> 2012-11-07  Manuel López-Ibáñez  <manu@gcc.gnu.org>
>
>       * optc-gen.awk: Factor code out to...
>       * opt-functions.awk (lang_enabled_by): ... this new function.
>
> Modified:
>     trunk/gcc/ChangeLog
>     trunk/gcc/opt-functions.awk
>     trunk/gcc/optc-gen.awk

I'm generally very much in favor of abstracting functionality into
separate functions.  Here, however, there ever existed only one user of
'gcc/opt-functions.awk:lang_enabled_by', its interface is a bit clumsy,
leading to (slightly) confusing diagnostics, and all handling for other
related option properties remains in the original place, so I find that
in fact confusing to be split out, and scattered over two files.  I thus
propose to merge 'gcc/opt-functions.awk:lang_enabled_by' back into its
original place, and then improve diagnostics.  OK to push the attached
"options: Improve 'LangEnabledBy' option property diagnostics"?  In
addition to standard testing, I've manually verified the error cases.


Grüße
 Thomas


-----------------
Siemens Electronic Design Automation GmbH; Anschrift: Arnulfstraße 201, 80634 München; Gesellschaft mit beschränkter Haftung; Geschäftsführer: Thomas Heurung, Frank Thürauf; Sitz der Gesellschaft: München; Registergericht München, HRB 106955

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-options-Improve-LangEnabledBy-option-property-diagno.patch --]
[-- Type: text/x-diff, Size: 5797 bytes --]

From 08eb8eca7673d2ce13ad22fd7e8823f5864e740e Mon Sep 17 00:00:00 2001
From: Thomas Schwinge <thomas@codesourcery.com>
Date: Mon, 28 Mar 2022 10:55:49 +0200
Subject: [PATCH] options: Improve 'LangEnabledBy' option property diagnostics

No changes in generated files.

	gcc/
	* opt-functions.awk (n_args): New function.
	(lang_enabled_by): Merge function into...
	* optc-gen.awk <END>: ... sole user here.
	Improve diagnostics.
---
 gcc/opt-functions.awk | 47 +++++++++---------------------------
 gcc/optc-gen.awk      | 56 +++++++++++++++++++++++++++++++++++++++----
 2 files changed, 62 insertions(+), 41 deletions(-)

diff --git a/gcc/opt-functions.awk b/gcc/opt-functions.awk
index 9eccf9b0409..2aee0b9f1c3 100644
--- a/gcc/opt-functions.awk
+++ b/gcc/opt-functions.awk
@@ -82,6 +82,17 @@ function opt_args_non_empty(name, flags, description)
 	return args
 }
 
+# Return the number of comma-separated element of S.
+function n_args(s)
+{
+	n = 1
+	while (s ~ ",") {
+		n++
+		sub("[^,]*, *", "", s)
+	}
+	return n
+}
+
 # Return the Nth comma-separated element of S.  Return the empty string
 # if S does not contain N elements.
 function nth_arg(n, s)
@@ -376,39 +387,3 @@ function integer_range_info(range_option, init, option, uinteger_used)
     else
         return "-1, -1"
 }
-
-# Handle LangEnabledBy(ENABLED_BY_LANGS, ENABLEDBY_NAME, ENABLEDBY_POSARG,
-# ENABLEDBY_NEGARG). This function does not return anything.
-function lang_enabled_by(enabledby_langs, enabledby_name, enabledby_posarg, enabledby_negarg)
-{
-    n_enabledby_arg_langs = split(enabledby_langs, enabledby_arg_langs, " ");
-    if (enabledby_posarg != "" && enabledby_negarg != "") {
-        with_args = "," enabledby_posarg "," enabledby_negarg
-    } else if (enabledby_posarg == "" && enabledby_negarg == "") {
-        with_args = ""
-    } else {
-        print "#error " opts[i] " LangEnabledBy("enabledby_langs","enabledby_name", " \
-            enabledby_posarg", " enabledby_negarg \
-            ") with three arguments, it should have either 2 or 4"
-    }
-
-    n_enabledby_array = split(enabledby_name, enabledby_array, " \\|\\| ");
-    for (k = 1; k <= n_enabledby_array; k++) {
-        enabledby_index = opt_numbers[enabledby_array[k]];
-        if (enabledby_index == "") {
-             print "#error " opts[i] " LangEnabledBy("enabledby_langs","enabledby_name", " \
-                 enabledby_posarg", " enabledby_negarg"), unknown option '" enabledby_name "'"
-        } else {
-            for (j = 1; j <= n_enabledby_arg_langs; j++) {
-                 lang_name = lang_sanitized_name(enabledby_arg_langs[j]);
-                 lang_index = lang_numbers[enabledby_arg_langs[j]];
-                 if (enables[lang_name,enabledby_array[k]] == "") {
-                     enabledby[lang_name,n_enabledby_lang[lang_index]] = enabledby_array[k];
-                     n_enabledby_lang[lang_index]++;
-                 }
-                 enables[lang_name,enabledby_array[k]] \
-                     = enables[lang_name,enabledby_array[k]] opts[i] with_args ";";
-            }
-        }
-    }
-}
diff --git a/gcc/optc-gen.awk b/gcc/optc-gen.awk
index 5f7946cf49b..f2198b253ad 100644
--- a/gcc/optc-gen.awk
+++ b/gcc/optc-gen.awk
@@ -98,11 +98,57 @@ for (i = 0; i < n_opts; i++) {
 
     enabledby_arg = opt_args("LangEnabledBy", flags[i]);
     if (enabledby_arg != "") {
-        enabledby_langs = nth_arg(0, enabledby_arg);
-        enabledby_name = nth_arg(1, enabledby_arg);
-        enabledby_posarg = nth_arg(2, enabledby_arg);
-	enabledby_negarg = nth_arg(3, enabledby_arg);
-        lang_enabled_by(enabledby_langs, enabledby_name, enabledby_posarg, enabledby_negarg);
+	enabledby_n_args = n_args(enabledby_arg)
+	if (enabledby_n_args != 2 \
+	    && enabledby_n_args != 4) {
+	    print "#error " opts[i] " LangEnabledBy(" enabledby_arg ") must specify two or four arguments"
+	}
+
+	enabledby_langs = nth_arg(0, enabledby_arg);
+	if (enabledby_langs == "")
+	    print "#error " opts[i] " LangEnabledBy(" enabledby_arg ") must specify LANGUAGE"
+	enabledby_opt = nth_arg(1, enabledby_arg);
+	if (enabledby_opt == "")
+	    print "#error " opts[i] " LangEnabledBy(" enabledby_arg ") must specify OPT"
+
+	enabledby_posarg_negarg = ""
+	if (enabledby_n_args == 4) {
+	    enabledby_posarg = nth_arg(2, enabledby_arg);
+	    enabledby_negarg = nth_arg(3, enabledby_arg);
+	    if (enabledby_posarg == "" \
+		|| enabledby_negarg == "")
+		print "#error " opts[i] " LangEnabledBy(" enabledby_arg ") with four arguments must specify POSARG and NEGARG"
+	    else
+		enabledby_posarg_negarg = "," enabledby_posarg "," enabledby_negarg
+	}
+
+	n_enabledby_arg_langs = split(enabledby_langs, enabledby_arg_langs, " ");
+	n_enabledby_array = split(enabledby_opt, enabledby_array, " \\|\\| ");
+	for (k = 1; k <= n_enabledby_array; k++) {
+	    enabledby_index = opt_numbers[enabledby_array[k]];
+	    if (enabledby_index == "") {
+		print "#error " opts[i] " LangEnabledBy(" enabledby_arg "), unknown option '" enabledby_opt "'"
+		continue
+	    }
+
+	    for (j = 1; j <= n_enabledby_arg_langs; j++) {
+		lang_name = enabledby_arg_langs[j]
+		lang_index = lang_numbers[lang_name];
+		if (lang_index == "") {
+		    print "#error " opts[i] " LangEnabledBy(" enabledby_arg "), unknown language '" lang_name "'"
+		    continue
+		}
+
+		lang_name = lang_sanitized_name(lang_name);
+
+		if (enables[lang_name,enabledby_array[k]] == "") {
+		    enabledby[lang_name,n_enabledby_lang[lang_index]] = enabledby_array[k];
+		    n_enabledby_lang[lang_index]++;
+		}
+		enables[lang_name,enabledby_array[k]] \
+		    = enables[lang_name,enabledby_array[k]] opts[i] enabledby_posarg_negarg ";";
+	    }
+	}
     }
 
     if (flag_set_p("Param", flags[i]) && !(opts[i] ~ "^-param="))
-- 
2.25.1


^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: options: Improve 'LangEnabledBy' option property diagnostics (was: r193303 - in /trunk/gcc: ChangeLog opt-function...)
  2022-03-29  9:28 ` options: Improve " Thomas Schwinge
@ 2022-03-29 19:37   ` Joseph Myers
  0 siblings, 0 replies; 3+ messages in thread
From: Joseph Myers @ 2022-03-29 19:37 UTC (permalink / raw)
  To: Thomas Schwinge; +Cc: manu, gcc-patches

On Tue, 29 Mar 2022, Thomas Schwinge wrote:

> I'm generally very much in favor of abstracting functionality into
> separate functions.  Here, however, there ever existed only one user of
> 'gcc/opt-functions.awk:lang_enabled_by', its interface is a bit clumsy,
> leading to (slightly) confusing diagnostics, and all handling for other
> related option properties remains in the original place, so I find that
> in fact confusing to be split out, and scattered over two files.  I thus
> propose to merge 'gcc/opt-functions.awk:lang_enabled_by' back into its
> original place, and then improve diagnostics.  OK to push the attached
> "options: Improve 'LangEnabledBy' option property diagnostics"?  In
> addition to standard testing, I've manually verified the error cases.

OK.

-- 
Joseph S. Myers
joseph@codesourcery.com

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2022-03-29 19:37 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [not found] <20121107181102.28015.qmail@sourceware.org>
2022-03-29  9:21 ` options: Fix 'enabledby_negargs' typo in 'LangEnabledBy' option property diagnostics (was: r193303 - in /trunk/gcc: ChangeLog opt-function...) Thomas Schwinge
2022-03-29  9:28 ` options: Improve " Thomas Schwinge
2022-03-29 19:37   ` Joseph Myers

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).