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.
Thanks,
Andrew
> + }
>
> - if ((TREE_CODE (decl) == FUNCTION_DECL
> - || TREE_CODE (decl) == VAR_DECL)
> - && TREE_CODE (TREE_VALUE (args)) == STRING_CST)
> - {
> - if (TREE_CODE (decl) == VAR_DECL
> - && current_function_decl != NULL_TREE
> - && !TREE_STATIC (decl))
> - {
> - error_at (DECL_SOURCE_LOCATION (decl),
> - "section attribute cannot be specified for "
> - "local variables");
> - *no_add_attrs = true;
> - }
> + user_defined_section_attribute = true;
>
> - /* The decl may have already been given a section attribute
> - from a previous declaration. Ensure they match. */
> - else if (DECL_SECTION_NAME (decl) != NULL
> - && strcmp (DECL_SECTION_NAME (decl),
> - TREE_STRING_POINTER (TREE_VALUE (args))) != 0)
> - {
> - error ("section of %q+D conflicts with previous declaration",
> - *node);
> - *no_add_attrs = true;
> - }
> - else if (TREE_CODE (decl) == VAR_DECL
> - && !targetm.have_tls && targetm.emutls.tmpl_section
> - && DECL_THREAD_LOCAL_P (decl))
> - {
> - error ("section of %q+D cannot be overridden", *node);
> - *no_add_attrs = true;
> - }
> - else
> - set_decl_section_name (decl,
> - TREE_STRING_POINTER (TREE_VALUE (args)));
> - }
> - else
> - {
> - error ("section attribute not allowed for %q+D", *node);
> - *no_add_attrs = true;
> - }
> + if (TREE_CODE (decl) != FUNCTION_DECL && TREE_CODE (decl) != VAR_DECL)
> + {
> + error ("section attribute not allowed for %q+D", *node);
> + goto fail;
> }
> - else
> +
> + if (TREE_CODE (TREE_VALUE (args)) != STRING_CST)
> {
> - error_at (DECL_SOURCE_LOCATION (*node),
> - "section attributes are not supported for this target");
> - *no_add_attrs = true;
> + error ("section attribute argument not a string constant");
> + goto fail;
> + }
> +
> + if (TREE_CODE (decl) == VAR_DECL
> + && current_function_decl != NULL_TREE
> + && !TREE_STATIC (decl))
> + {
> + error_at (DECL_SOURCE_LOCATION (decl),
> + "section attribute cannot be specified for local variables");
> + goto fail;
> }
>
> + /* The decl may have already been given a section attribute
> + from a previous declaration. Ensure they match. */
> + if (DECL_SECTION_NAME (decl) != NULL
> + && strcmp (DECL_SECTION_NAME (decl),
> + TREE_STRING_POINTER (TREE_VALUE (args))) != 0)
> + {
> + error ("section of %q+D conflicts with previous declaration", *node);
> + goto fail;
> + }
> +
> + if (TREE_CODE (decl) == VAR_DECL
> + && !targetm.have_tls && targetm.emutls.tmpl_section
> + && DECL_THREAD_LOCAL_P (decl))
> + {
> + error ("section of %q+D cannot be overridden", *node);
> + goto fail;
> + }
> +
> + set_decl_section_name (decl, TREE_STRING_POINTER (TREE_VALUE (args)));
> + return NULL_TREE;
> +
> +fail:
> + *no_add_attrs = true;
> return NULL_TREE;
> }
>
> --
> 2.1.0
>
