* c-family/c-pretty-print.c - fix for 'restrict' quliafiers
@ 2015-08-17 2:35 Gary Funck
2015-08-17 10:49 ` Marek Polacek
0 siblings, 1 reply; 5+ messages in thread
From: Gary Funck @ 2015-08-17 2:35 UTC (permalink / raw)
To: Gcc Patches; +Cc: Nenad Vukicevic
While reviewing some code, I noticed that the logic for
pretty-printing 'restrict' qualifiers is likely missing a
statement that sets 'previous'.
OK to commit?
2015-08-l6 Gary Funck <gary@intrepid.com>
* c-pretty-print.c (pp_c_cv_qualifiers):
Set 'previous' for restrict qualifiers.
Index: c-pretty-print.c
===================================================================
--- c-pretty-print.c (revision 226928)
+++ c-pretty-print.c (working copy)
@@ -207,16 +207,17 @@ pp_c_cv_qualifiers (c_pretty_printer *pp
}
if (qualifiers & TYPE_QUAL_RESTRICT)
{
if (previous)
pp_c_whitespace (pp);
pp_c_ws_string (pp, (flag_isoc99 && !c_dialect_cxx ()
? "restrict" : "__restrict__"));
+ previous = true;
}
}
/* Pretty-print T using the type-cast notation '( type-name )'. */
static void
pp_c_type_cast (c_pretty_printer *pp, tree t)
{
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: c-family/c-pretty-print.c - fix for 'restrict' quliafiers 2015-08-17 2:35 c-family/c-pretty-print.c - fix for 'restrict' quliafiers Gary Funck @ 2015-08-17 10:49 ` Marek Polacek 2015-08-17 12:46 ` Gary Funck 2015-08-17 13:12 ` Marek Polacek 0 siblings, 2 replies; 5+ messages in thread From: Marek Polacek @ 2015-08-17 10:49 UTC (permalink / raw) To: Gary Funck; +Cc: Gcc Patches, Nenad Vukicevic On Sun, Aug 16, 2015 at 06:14:18PM -0700, Gary Funck wrote: > > While reviewing some code, I noticed that the logic for > pretty-printing 'restrict' qualifiers is likely missing a > statement that sets 'previous'. > > OK to commit? > > 2015-08-l6 Gary Funck <gary@intrepid.com> > > * c-pretty-print.c (pp_c_cv_qualifiers): > Set 'previous' for restrict qualifiers. > > Index: c-pretty-print.c > =================================================================== > --- c-pretty-print.c (revision 226928) > +++ c-pretty-print.c (working copy) > @@ -207,16 +207,17 @@ pp_c_cv_qualifiers (c_pretty_printer *pp > } > > if (qualifiers & TYPE_QUAL_RESTRICT) > { > if (previous) > pp_c_whitespace (pp); > pp_c_ws_string (pp, (flag_isoc99 && !c_dialect_cxx () > ? "restrict" : "__restrict__")); > + previous = true; No, I don't think this assignment is missing here. The restrict qualifier is printed last so we don't need to mark that we've printed something. Actually, the whole "previous" flag seems to be redundant; pp_c_ws_string calls pp_c_maybe_whitespace so it prints a whitespace if necessary. So I suggest the following instead (haven't tested it yet). 2015-08-17 Marek Polacek <polacek@redhat.com> * c-pretty-print.c (pp_c_cv_qualifiers): Remove code dealing with whitespaces before qualifier names. diff --git gcc/c-family/c-pretty-print.c gcc/c-family/c-pretty-print.c index 90f8c3d..e2809cf 100644 --- gcc/c-family/c-pretty-print.c +++ gcc/c-family/c-pretty-print.c @@ -173,7 +173,6 @@ void pp_c_cv_qualifiers (c_pretty_printer *pp, int qualifiers, bool func_type) { const char *p = pp_last_position_in_text (pp); - bool previous = false; if (!qualifiers) return; @@ -185,34 +184,14 @@ pp_c_cv_qualifiers (c_pretty_printer *pp, int qualifiers, bool func_type) pp_c_whitespace (pp); if (qualifiers & TYPE_QUAL_ATOMIC) - { - pp_c_ws_string (pp, "_Atomic"); - previous = true; - } - + pp_c_ws_string (pp, "_Atomic"); if (qualifiers & TYPE_QUAL_CONST) - { - if (previous) - pp_c_whitespace (pp); - pp_c_ws_string (pp, func_type ? "__attribute__((const))" : "const"); - previous = true; - } - + pp_c_ws_string (pp, func_type ? "__attribute__((const))" : "const"); if (qualifiers & TYPE_QUAL_VOLATILE) - { - if (previous) - pp_c_whitespace (pp); - pp_c_ws_string (pp, func_type ? "__attribute__((noreturn))" : "volatile"); - previous = true; - } - + pp_c_ws_string (pp, func_type ? "__attribute__((noreturn))" : "volatile"); if (qualifiers & TYPE_QUAL_RESTRICT) - { - if (previous) - pp_c_whitespace (pp); - pp_c_ws_string (pp, (flag_isoc99 && !c_dialect_cxx () - ? "restrict" : "__restrict__")); - } + pp_c_ws_string (pp, (flag_isoc99 && !c_dialect_cxx () + ? "restrict" : "__restrict__")); } /* Pretty-print T using the type-cast notation '( type-name )'. */ Marek ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: c-family/c-pretty-print.c - fix for 'restrict' quliafiers 2015-08-17 10:49 ` Marek Polacek @ 2015-08-17 12:46 ` Gary Funck 2015-08-17 13:12 ` Marek Polacek 1 sibling, 0 replies; 5+ messages in thread From: Gary Funck @ 2015-08-17 12:46 UTC (permalink / raw) To: Marek Polacek; +Cc: Gcc Patches, Nenad Vukicevic On 08/17/15 12:06:08, Marek Polacek wrote: > No, I don't think this assignment is missing here. The restrict qualifier > is printed last so we don't need to mark that we've printed something. > > Actually, the whole "previous" flag seems to be redundant; pp_c_ws_string > calls pp_c_maybe_whitespace so it prints a whitespace if necessary. OK. I'm not familiar with this code, so please go ahead and make the changes as needed. - Gary ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: c-family/c-pretty-print.c - fix for 'restrict' quliafiers 2015-08-17 10:49 ` Marek Polacek 2015-08-17 12:46 ` Gary Funck @ 2015-08-17 13:12 ` Marek Polacek 2015-08-17 14:14 ` Jason Merrill 1 sibling, 1 reply; 5+ messages in thread From: Marek Polacek @ 2015-08-17 13:12 UTC (permalink / raw) To: Gary Funck; +Cc: Gcc Patches, Nenad Vukicevic, Jason Merrill, Joseph Myers On Mon, Aug 17, 2015 at 12:06:08PM +0200, Marek Polacek wrote: > On Sun, Aug 16, 2015 at 06:14:18PM -0700, Gary Funck wrote: > > > > While reviewing some code, I noticed that the logic for > > pretty-printing 'restrict' qualifiers is likely missing a > > statement that sets 'previous'. > > > > OK to commit? > > > > 2015-08-l6 Gary Funck <gary@intrepid.com> > > > > * c-pretty-print.c (pp_c_cv_qualifiers): > > Set 'previous' for restrict qualifiers. > > > > Index: c-pretty-print.c > > =================================================================== > > --- c-pretty-print.c (revision 226928) > > +++ c-pretty-print.c (working copy) > > @@ -207,16 +207,17 @@ pp_c_cv_qualifiers (c_pretty_printer *pp > > } > > > > if (qualifiers & TYPE_QUAL_RESTRICT) > > { > > if (previous) > > pp_c_whitespace (pp); > > pp_c_ws_string (pp, (flag_isoc99 && !c_dialect_cxx () > > ? "restrict" : "__restrict__")); > > + previous = true; > > No, I don't think this assignment is missing here. The restrict qualifier > is printed last so we don't need to mark that we've printed something. > > Actually, the whole "previous" flag seems to be redundant; pp_c_ws_string > calls pp_c_maybe_whitespace so it prints a whitespace if necessary. > > So I suggest the following instead (haven't tested it yet). Now regtested/bootstrapped on x86_64-linux. Jason/Joseph, ok? > 2015-08-17 Marek Polacek <polacek@redhat.com> > > * c-pretty-print.c (pp_c_cv_qualifiers): Remove code dealing > with whitespaces before qualifier names. > > diff --git gcc/c-family/c-pretty-print.c gcc/c-family/c-pretty-print.c > index 90f8c3d..e2809cf 100644 > --- gcc/c-family/c-pretty-print.c > +++ gcc/c-family/c-pretty-print.c > @@ -173,7 +173,6 @@ void > pp_c_cv_qualifiers (c_pretty_printer *pp, int qualifiers, bool func_type) > { > const char *p = pp_last_position_in_text (pp); > - bool previous = false; > > if (!qualifiers) > return; > @@ -185,34 +184,14 @@ pp_c_cv_qualifiers (c_pretty_printer *pp, int qualifiers, bool func_type) > pp_c_whitespace (pp); > > if (qualifiers & TYPE_QUAL_ATOMIC) > - { > - pp_c_ws_string (pp, "_Atomic"); > - previous = true; > - } > - > + pp_c_ws_string (pp, "_Atomic"); > if (qualifiers & TYPE_QUAL_CONST) > - { > - if (previous) > - pp_c_whitespace (pp); > - pp_c_ws_string (pp, func_type ? "__attribute__((const))" : "const"); > - previous = true; > - } > - > + pp_c_ws_string (pp, func_type ? "__attribute__((const))" : "const"); > if (qualifiers & TYPE_QUAL_VOLATILE) > - { > - if (previous) > - pp_c_whitespace (pp); > - pp_c_ws_string (pp, func_type ? "__attribute__((noreturn))" : "volatile"); > - previous = true; > - } > - > + pp_c_ws_string (pp, func_type ? "__attribute__((noreturn))" : "volatile"); > if (qualifiers & TYPE_QUAL_RESTRICT) > - { > - if (previous) > - pp_c_whitespace (pp); > - pp_c_ws_string (pp, (flag_isoc99 && !c_dialect_cxx () > - ? "restrict" : "__restrict__")); > - } > + pp_c_ws_string (pp, (flag_isoc99 && !c_dialect_cxx () > + ? "restrict" : "__restrict__")); > } > > /* Pretty-print T using the type-cast notation '( type-name )'. */ > > Marek Marek ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: c-family/c-pretty-print.c - fix for 'restrict' quliafiers 2015-08-17 13:12 ` Marek Polacek @ 2015-08-17 14:14 ` Jason Merrill 0 siblings, 0 replies; 5+ messages in thread From: Jason Merrill @ 2015-08-17 14:14 UTC (permalink / raw) To: Marek Polacek, Gary Funck; +Cc: Gcc Patches, Nenad Vukicevic, Joseph Myers OK. Jason ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2015-08-17 14:13 UTC | newest] Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2015-08-17 2:35 c-family/c-pretty-print.c - fix for 'restrict' quliafiers Gary Funck 2015-08-17 10:49 ` Marek Polacek 2015-08-17 12:46 ` Gary Funck 2015-08-17 13:12 ` Marek Polacek 2015-08-17 14:14 ` Jason Merrill
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).