| 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.
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
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).
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
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 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
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