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