| Message ID | yddziiv86ag.fsf@CeBiTec.Uni-Bielefeld.DE |
|---|---|
| State | New |
| Headers | show |
On Fri, 13 Jan 2017, Rainer Orth wrote: > I'm unsure if the patch is large enough to need a copyright assignment > (in which case it's almost certainly too late for GCC 7), and even if > not if it's appropriate at this point in the release cycle. I think it's big enough to need an assignment. Note missing spaces before '(' in calls to free, and after ')' in a cast. -- Joseph S. Myers joseph@codesourcery.com
Hi Joseph, > On Fri, 13 Jan 2017, Rainer Orth wrote: > >> I'm unsure if the patch is large enough to need a copyright assignment >> (in which case it's almost certainly too late for GCC 7), and even if >> not if it's appropriate at this point in the release cycle. > > I think it's big enough to need an assignment. ok, then I'll get the ball rolling. Are the forms on https://www.gnu.org/software/gnulib/Copyright/ current? I didn't have to deal with the copyright assignment process so far. > Note missing spaces before '(' in calls to free, and after ')' in a cast. Thanks, will fix. Rainer -- ----------------------------------------------------------------------------- Rainer Orth, Center for Biotechnology, Bielefeld University
On Fri, 13 Jan 2017, Rainer Orth wrote: > ok, then I'll get the ball rolling. Are the forms on > > https://www.gnu.org/software/gnulib/Copyright/ > > current? I didn't have to deal with the copyright assignment process so > far. As far as I know they are (the place I refer people to is actually gnulib git rather than the gnulib website). -- Joseph S. Myers joseph@codesourcery.com
On 01/13/2017 05:59 AM, Rainer Orth wrote: > diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi > --- a/gcc/doc/invoke.texi > +++ b/gcc/doc/invoke.texi > @@ -26391,6 +26391,13 @@ be as many clauses as you need. This ma > > @end table > > +The switch matching text @code{S} in a %@{@code{S}@}, > +%@{@code{S}:@code{X}@} or similar construct can use a backslash to > +ignore the special meaning of the character following it, thus allowing > +literal matching of a character that is otherwise specially treated. > +For example, %@{@code{std=iso9899\:1999}:@code{X}@} would substitute > +@code{X} if the @option{-std=iso9899:1999} option were given. > + I see this "%@{@code{..." markup appears in the paragraph just before this, but it's wrong. The whole thing needs to be wrapped in @samp and the nested @codes removed, like s/%@{@code{S}:@code{X}@}/@samp{%@{S:X@}}/ etc. I also suggest using the present tense here instead of the subjunctive... s/would substitute/substitutes/ s/were given/is given/ -Sandra
Hi Sandra, > On 01/13/2017 05:59 AM, Rainer Orth wrote: >> diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi >> --- a/gcc/doc/invoke.texi >> +++ b/gcc/doc/invoke.texi >> @@ -26391,6 +26391,13 @@ be as many clauses as you need. This ma >> >> @end table >> >> +The switch matching text @code{S} in a %@{@code{S}@}, >> +%@{@code{S}:@code{X}@} or similar construct can use a backslash to >> +ignore the special meaning of the character following it, thus allowing >> +literal matching of a character that is otherwise specially treated. >> +For example, %@{@code{std=iso9899\:1999}:@code{X}@} would substitute >> +@code{X} if the @option{-std=iso9899:1999} option were given. >> + > > I see this "%@{@code{..." markup appears in the paragraph just before this, > but it's wrong. The whole thing needs to be wrapped in @samp and the > nested @codes removed, like > > s/%@{@code{S}:@code{X}@}/@samp{%@{S:X@}}/ > > etc. I see, fixed. I assume this applies to the uses inside @item, too, and irrespective of %{S:X} or %{S}? > I also suggest using the present tense here instead of the subjunctive... > > s/would substitute/substitutes/ > s/were given/is given/ Makes sense: fixed both in gcc.c and invoke.texi. Thanks. Rainer -- ----------------------------------------------------------------------------- Rainer Orth, Center for Biotechnology, Bielefeld University
On 01/16/2017 03:54 AM, Rainer Orth wrote: > Hi Sandra, > >> On 01/13/2017 05:59 AM, Rainer Orth wrote: >>> diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi >>> --- a/gcc/doc/invoke.texi >>> +++ b/gcc/doc/invoke.texi >>> @@ -26391,6 +26391,13 @@ be as many clauses as you need. This ma >>> >>> @end table >>> >>> +The switch matching text @code{S} in a %@{@code{S}@}, >>> +%@{@code{S}:@code{X}@} or similar construct can use a backslash to >>> +ignore the special meaning of the character following it, thus allowing >>> +literal matching of a character that is otherwise specially treated. >>> +For example, %@{@code{std=iso9899\:1999}:@code{X}@} would substitute >>> +@code{X} if the @option{-std=iso9899:1999} option were given. >>> + >> >> I see this "%@{@code{..." markup appears in the paragraph just before this, >> but it's wrong. The whole thing needs to be wrapped in @samp and the >> nested @codes removed, like >> >> s/%@{@code{S}:@code{X}@}/@samp{%@{S:X@}}/ >> >> etc. > > I see, fixed. I assume this applies to the uses inside @item, too, and > irrespective of %{S:X} or %{S}? It looked to me like this table environment uses @table @code so there should be no need for additional @code markup within the @item tags. I'm not asking you to repair existing bad markup elsewhere in this section, just not propagate it to your new paragraph of text. -Sandra
Hi Joseph, > On Fri, 13 Jan 2017, Rainer Orth wrote: > >> I'm unsure if the patch is large enough to need a copyright assignment >> (in which case it's almost certainly too late for GCC 7), and even if >> not if it's appropriate at this point in the release cycle. > > I think it's big enough to need an assignment. Jeff informed me that he'd received confirmation from the FSF that his assignment has been processed. > Note missing spaces before '(' in calls to free, and after ')' in a cast. Fixed in the following revised patch, which also incorporates Sandra's review comments. Bootstrapped without regressions on i386-pc-solaris2.12 and x86_64-pc-linux-gnu, in a tree which also contained the patch for PR target/40411 which needs this feature. The doc parts have been checked separately with $ make doc/gcc.info doc/gcc.pdf and visual inspection of the output. Ok for mainline either now or when GCC 8 stage 1 opens? Thanks. Rainer -- ----------------------------------------------------------------------------- Rainer Orth, Center for Biotechnology, Bielefeld University 2017-01-10 Jeff Downs <heydowns@somuchpressure.net> Rainer Orth <ro@CeBiTec.Uni-Bielefeld.DE> * gcc.c (handle_braces): Support escaping in switch matching text. * doc/invoke.texi (Spec Files): Document it. Remove superfluous @code markup in items.# HG changeset patch # Parent 12dd51d3fddd52cbb3925fe2a7950cb8df0ad230 Support escaping special characters in specs diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi --- a/gcc/doc/invoke.texi +++ b/gcc/doc/invoke.texi @@ -26207,7 +26207,7 @@ Substitute the variable part of a matche Note that each comma in the substituted string is replaced by a single space. -@item %<@code{S} +@item %<S Remove all occurrences of @code{-S} from the command line. Note---this command is position dependent. @samp{%} commands in the spec string before this one see @code{-S}, @samp{%} commands in the spec string @@ -26305,7 +26305,7 @@ It is used to separate compiler options in the @option{--target-help} output. @end table -@item %@{@code{S}@} +@item %@{S@} Substitutes the @code{-S} switch, if that switch is given to GCC@. If that switch is not specified, this substitutes nothing. Note that the leading dash is omitted when specifying this option, and it is @@ -26313,11 +26313,11 @@ automatically inserted if the substituti string @samp{%@{foo@}} matches the command-line option @option{-foo} and outputs the command-line option @option{-foo}. -@item %W@{@code{S}@} +@item %W@{S@} Like %@{@code{S}@} but mark last argument supplied within as a file to be deleted on failure. -@item %@{@code{S}*@} +@item %@{S*@} Substitutes all the switches specified to GCC whose names start with @code{-S}, but which also take an argument. This is used for switches like @option{-o}, @option{-D}, @option{-I}, etc. @@ -26325,19 +26325,19 @@ GCC considers @option{-o foo} as being one switch whose name starts with @samp{o}. %@{o*@} substitutes this text, including the space. Thus two arguments are generated. -@item %@{@code{S}*&@code{T}*@} +@item %@{S*&T*@} Like %@{@code{S}*@}, but preserve order of @code{S} and @code{T} options (the order of @code{S} and @code{T} in the spec is not significant). There can be any number of ampersand-separated variables; for each the wild card is optional. Useful for CPP as @samp{%@{D*&U*&A*@}}. -@item %@{@code{S}:@code{X}@} +@item %@{S:X@} Substitutes @code{X}, if the @option{-S} switch is given to GCC@. -@item %@{!@code{S}:@code{X}@} +@item %@{!S:X@} Substitutes @code{X}, if the @option{-S} switch is @emph{not} given to GCC@. -@item %@{@code{S}*:@code{X}@} +@item %@{S*:X@} Substitutes @code{X} if one or more switches whose names start with @code{-S} are specified to GCC@. Normally @code{X} is substituted only once, no matter how many such switches appeared. However, if @code{%*} @@ -26362,19 +26362,19 @@ when matching an option like @option{-mc --script=newchip/memory.ld @end smallexample -@item %@{.@code{S}:@code{X}@} +@item %@{.S:X@} Substitutes @code{X}, if processing a file with suffix @code{S}. -@item %@{!.@code{S}:@code{X}@} +@item %@{!.S:X@} Substitutes @code{X}, if @emph{not} processing a file with suffix @code{S}. -@item %@{,@code{S}:@code{X}@} +@item %@{,S:X@} Substitutes @code{X}, if processing a file for language @code{S}. -@item %@{!,@code{S}:@code{X}@} +@item %@{!,S:X@} Substitutes @code{X}, if not processing a file for language @code{S}. -@item %@{@code{S}|@code{P}:@code{X}@} +@item %@{S|P:X@} Substitutes @code{X} if either @code{-S} or @code{-P} is given to GCC@. This may be combined with @samp{!}, @samp{.}, @samp{,}, and @code{*} sequences as well, although they have a stronger binding than @@ -26409,7 +26409,14 @@ be as many clauses as you need. This ma @end table -The conditional text @code{X} in a %@{@code{S}:@code{X}@} or similar +The switch matching text @code{S} in a @samp{%@{S@}}, @samp{%@{S:X@}} +or similar construct can use a backslash to ignore the special meaning +of the character following it, thus allowing literal matching of a +character that is otherwise specially treated. For example, +@samp{%@{std=iso9899\:1999:X@}} substitutes @code{X} if the +@option{-std=iso9899:1999} option is given. + +The conditional text @code{X} in a @samp{%@{S:X@}} or similar construct may contain other nested @samp{%} constructs or spaces, or even newlines. They are processed as usual, as described above. Trailing white space in @code{X} is ignored. White space may also diff --git a/gcc/gcc.c b/gcc/gcc.c --- a/gcc/gcc.c +++ b/gcc/gcc.c @@ -584,6 +584,12 @@ or with constant text in a single argume %(Spec) processes a specification defined in a specs file as *Spec: +The switch matching text S in a %{S}, %{S:X}, or similar construct can use +a backslash to ignore the special meaning of the character following it, +thus allowing literal matching of a character that is otherwise specially +treated. For example, %{std=iso9899\:1999:X} substitutes X if the +-std=iso9899:1999 option is given. + The conditional text X in a %{S:X} or similar construct may contain other nested % constructs or spaces, or even newlines. They are processed as usual, as described above. Trailing white space in X is @@ -6237,6 +6243,8 @@ handle_braces (const char *p) { const char *atom, *end_atom; const char *d_atom = NULL, *d_end_atom = NULL; + char *esc_buf = NULL, *d_esc_buf = NULL; + int esc; const char *orig = p; bool a_is_suffix; @@ -6287,11 +6295,42 @@ handle_braces (const char *p) p++, a_is_spectype = true; atom = p; + esc = 0; while (ISIDNUM (*p) || *p == '-' || *p == '+' || *p == '=' - || *p == ',' || *p == '.' || *p == '@') - p++; + || *p == ',' || *p == '.' || *p == '@' || *p == '\\') + { + if (*p == '\\') + { + p++; + if (!*p) + fatal_error (input_location, + "braced spec %qs ends in escape", orig); + esc++; + } + p++; + } end_atom = p; + if (esc) + { + const char *ap; + char *ep; + + if (esc_buf && esc_buf != d_esc_buf) + free (esc_buf); + esc_buf = NULL; + ep = esc_buf = (char *) xmalloc (end_atom - atom - esc + 1); + for (ap = atom; ap != end_atom; ap++, ep++) + { + if (*ap == '\\') + ap++; + *ep = *ap; + } + *ep = '\0'; + atom = esc_buf; + end_atom = ep; + } + if (*p == '*') p++, a_is_starred = 1; } @@ -6358,6 +6397,7 @@ handle_braces (const char *p) disj_matched = true; d_atom = atom; d_end_atom = end_atom; + d_esc_buf = esc_buf; } } } @@ -6369,7 +6409,7 @@ handle_braces (const char *p) p = process_brace_body (p + 1, d_atom, d_end_atom, disj_starred, disj_matched && !n_way_matched); if (p == 0) - return 0; + goto done; /* If we have an N-way choice, reset state for the next disjunction. */ @@ -6390,6 +6430,12 @@ handle_braces (const char *p) } while (*p++ != '}'); + done: + if (d_esc_buf && d_esc_buf != esc_buf) + free (d_esc_buf); + if (esc_buf) + free (esc_buf); + return p; invalid:
Hi Sandra, > On 01/16/2017 03:54 AM, Rainer Orth wrote: >> Hi Sandra, >> >>> On 01/13/2017 05:59 AM, Rainer Orth wrote: >>>> diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi >>>> --- a/gcc/doc/invoke.texi >>>> +++ b/gcc/doc/invoke.texi >>>> @@ -26391,6 +26391,13 @@ be as many clauses as you need. This ma >>>> >>>> @end table >>>> >>>> +The switch matching text @code{S} in a %@{@code{S}@}, >>>> +%@{@code{S}:@code{X}@} or similar construct can use a backslash to >>>> +ignore the special meaning of the character following it, thus allowing >>>> +literal matching of a character that is otherwise specially treated. >>>> +For example, %@{@code{std=iso9899\:1999}:@code{X}@} would substitute >>>> +@code{X} if the @option{-std=iso9899:1999} option were given. >>>> + >>> >>> I see this "%@{@code{..." markup appears in the paragraph just before this, >>> but it's wrong. The whole thing needs to be wrapped in @samp and the >>> nested @codes removed, like >>> >>> s/%@{@code{S}:@code{X}@}/@samp{%@{S:X@}}/ >>> >>> etc. >> >> I see, fixed. I assume this applies to the uses inside @item, too, and >> irrespective of %{S:X} or %{S}? > > It looked to me like this table environment uses > > @table @code > > so there should be no need for additional @code markup within the @item > tags. > > I'm not asking you to repair existing bad markup elsewhere in this section, > just not propagate it to your new paragraph of text. perhaps not, but it makes sense to fix this while I'm at this section. There are still more instances of the same problems (e.g. in the table listing the spec functions), but I haven't touched those. Should be all in the revised patch just submitted. Rainer -- ----------------------------------------------------------------------------- Rainer Orth, Center for Biotechnology, Bielefeld University
On 02/21/2017 06:54 AM, Rainer Orth wrote: > Hi Joseph, > >> On Fri, 13 Jan 2017, Rainer Orth wrote: >> >>> I'm unsure if the patch is large enough to need a copyright assignment >>> (in which case it's almost certainly too late for GCC 7), and even if >>> not if it's appropriate at this point in the release cycle. >> >> I think it's big enough to need an assignment. > > Jeff informed me that he'd received confirmation from the FSF that his > assignment has been processed. > >> Note missing spaces before '(' in calls to free, and after ')' in a cast. > > Fixed in the following revised patch, which also incorporates Sandra's > review comments. > > Bootstrapped without regressions on i386-pc-solaris2.12 and > x86_64-pc-linux-gnu, in a tree which also contained the patch for PR > target/40411 which needs this feature. > > The doc parts have been checked separately with > > $ make doc/gcc.info doc/gcc.pdf > > and visual inspection of the output. > > Ok for mainline either now or when GCC 8 stage 1 opens? The doc parts of the patch look OK for whenever the code change can go in. -Sandra
On Tue, 21 Feb 2017, Rainer Orth wrote: > 2017-01-10 Jeff Downs <heydowns@somuchpressure.net> > Rainer Orth <ro@CeBiTec.Uni-Bielefeld.DE> > > * gcc.c (handle_braces): Support escaping in switch matching > text. > * doc/invoke.texi (Spec Files): Document it. > Remove superfluous @code markup in items. OK. -- Joseph S. Myers joseph@codesourcery.com
diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi --- a/gcc/doc/invoke.texi +++ b/gcc/doc/invoke.texi @@ -26391,6 +26391,13 @@ be as many clauses as you need. This ma @end table +The switch matching text @code{S} in a %@{@code{S}@}, +%@{@code{S}:@code{X}@} or similar construct can use a backslash to +ignore the special meaning of the character following it, thus allowing +literal matching of a character that is otherwise specially treated. +For example, %@{@code{std=iso9899\:1999}:@code{X}@} would substitute +@code{X} if the @option{-std=iso9899:1999} option were given. + The conditional text @code{X} in a %@{@code{S}:@code{X}@} or similar construct may contain other nested @samp{%} constructs or spaces, or even newlines. They are processed as usual, as described above. diff --git a/gcc/gcc.c b/gcc/gcc.c --- a/gcc/gcc.c +++ b/gcc/gcc.c @@ -583,6 +583,12 @@ or with constant text in a single argume %(Spec) processes a specification defined in a specs file as *Spec: +The switch matching text S in a %{S}, %{S:X}, or similar construct can use a +backslash to ignore the special meaning of the character following it, thus +allowing literal matching of a character that is otherwise specially treated. +For example, %{std=iso9899\:1999:X} would substitute X if the +-std=iso9899:1999 option were given. + The conditional text X in a %{S:X} or similar construct may contain other nested % constructs or spaces, or even newlines. They are processed as usual, as described above. Trailing white space in X is @@ -6228,6 +6234,8 @@ handle_braces (const char *p) { const char *atom, *end_atom; const char *d_atom = NULL, *d_end_atom = NULL; + char *esc_buf = NULL, *d_esc_buf = NULL; + int esc; const char *orig = p; bool a_is_suffix; @@ -6278,11 +6286,41 @@ handle_braces (const char *p) p++, a_is_spectype = true; atom = p; + esc = 0; while (ISIDNUM (*p) || *p == '-' || *p == '+' || *p == '=' - || *p == ',' || *p == '.' || *p == '@') - p++; + || *p == ',' || *p == '.' || *p == '@' || *p == '\\') + { + if (*p == '\\') + { + p++; + if (!*p) + fatal_error (input_location, + "braced spec %qs ends in escape", orig); + esc++; + } + p++; + } end_atom = p; + if (esc) + { + const char *ap; + char *ep; + if (esc_buf && esc_buf != d_esc_buf) + free(esc_buf); + esc_buf = NULL; + ep = esc_buf = (char *)xmalloc (end_atom - atom - esc + 1); + for (ap = atom; ap != end_atom; ap++, ep++) + { + if (*ap == '\\') + ap++; + *ep = *ap; + } + *ep = '\0'; + atom = esc_buf; + end_atom = ep; + } + if (*p == '*') p++, a_is_starred = 1; } @@ -6349,6 +6387,7 @@ handle_braces (const char *p) disj_matched = true; d_atom = atom; d_end_atom = end_atom; + d_esc_buf = esc_buf; } } } @@ -6360,7 +6399,7 @@ handle_braces (const char *p) p = process_brace_body (p + 1, d_atom, d_end_atom, disj_starred, disj_matched && !n_way_matched); if (p == 0) - return 0; + goto done; /* If we have an N-way choice, reset state for the next disjunction. */ @@ -6381,6 +6420,12 @@ handle_braces (const char *p) } while (*p++ != '}'); + done: + if (d_esc_buf && d_esc_buf != esc_buf) + free(d_esc_buf); + if (esc_buf) + free(esc_buf); + return p; invalid:
As a prerequisite for (a corner case in) fixing PR target/40411, we need the ability to escape special characters in specs. Case at hand: we want to match -std=iso9899:199409 which doesn't work right now. The option isn't an alias but the canonical form (though in theory one could introduce -std=c94 and make the above an alias for that), so we can only match it directly. The goal is to have this work: #define STARTFILE_ARCH_SPEC \ "%{ansi|std=c90|std=iso9899\\:199409:values-Xc.o%s; :values-Xa.o%s} \ %{std=c90|std=gnu90:values-xpg4.o%s; :values-xpg6.o%s}" Jeff submitted a patch for this in the PR almost 8 years ago, and I've just updated it slightly so it applies to mainline, and copied the docs snippet to invoke.texi with the necessary markup. Bootstrapped (together with the current proposed patch to fix the PR above) on i386-pc-solaris2.12 and sparc-sun-solaris2.12. I'm unsure if the patch is large enough to need a copyright assignment (in which case it's almost certainly too late for GCC 7), and even if not if it's appropriate at this point in the release cycle. How should we deal with this? Thanks. Rainer -- ----------------------------------------------------------------------------- Rainer Orth, Center for Biotechnology, Bielefeld University 2017-01-10 Jeff Downs <heydowns@somuchpressure.net> Rainer Orth <ro@CeBiTec.Uni-Bielefeld.DE> gcc: * gcc.c (handle_braces): Support escaping in switch matching text. * doc/invoke.texi (Spec Files): Document it.