On Sun, Aug 24, 2014 at 01:58:52PM -0700, Andrew Pinski wrote:
> On Sun, Aug 24, 2014 at 1:42 PM, Josh Triplett <j...@joshtriplett.org> wrote:
> > handle_section_attribute contains many levels of nested conditionals and
> > branching code flow paths, with the error cases sometimes in the else
> > case and sometimes in the if case. Simplify the code flow into a series
> > of potential failure cases ending with the successful path, with no
> > nesting and no else clauses.
> > ---
> >
> > I started to work on an extension to the section attribute, and found
> > myself cleaning up the logic in handle_section_attribute; I wanted to
> > send this cleanup patch separately.
> >
> > I'm not a GCC committer, so I'm looking both for review and for someone
> > to commit this patch.
> >
> > gcc/ChangeLog | 5 +++
> > gcc/c-family/c-common.c | 91
> > +++++++++++++++++++++++++------------------------
> > 2 files changed, 51 insertions(+), 45 deletions(-)
> >
> > diff --git a/gcc/ChangeLog b/gcc/ChangeLog
> > index 703a489..0286bfb 100644
> > --- a/gcc/ChangeLog
> > +++ b/gcc/ChangeLog
> > @@ -1,3 +1,8 @@
> > +2014-08-24 Josh Triplett <j...@joshtriplett.org>
> > +
> > + * c-family/c-common.c (handle_section_attribute): Refactor to reduce
> > + nesting and distinguish between error cases.
> > +
> > 2014-08-24 Oleg Endo <olege...@gcc.gnu.org>
> >
> > PR target/61996
> > diff --git a/gcc/c-family/c-common.c b/gcc/c-family/c-common.c
> > index 58b9763..a63eedf 100644
> > --- a/gcc/c-family/c-common.c
> > +++ b/gcc/c-family/c-common.c
> > @@ -7387,58 +7387,59 @@ handle_section_attribute (tree *node, tree
> > ARG_UNUSED (name), tree args,
> > {
> > tree decl = *node;
> >
> > - if (targetm_common.have_named_sections)
> > + if (!targetm_common.have_named_sections)
> > {
> > - user_defined_section_attribute = true;
> > + error_at (DECL_SOURCE_LOCATION (*node),
> > + "section attributes are not supported for this target");
> > + goto fail;
>
> Why not move this to a different function and then do:
>
> if (function(...))
> set_decl_section_name (decl, TREE_STRING_POINTER (TREE_VALUE (args)));
> else
> *no_add_attrs = true;
>
> return NULL_TREE;
>
> This is a way to get away from using gotos.
I can certainly do so; I hadn't in this patch because the file in
question already contains dozens of instances of the same goto-based
error-handling pattern.
I'll send a second version of this patch with a separate function
returning bool.
- Josh Triplett
