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;
+    }
 
-      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

Reply via email to