This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Interlacing switch labels and compound statements



I stumbled over this code using a source code analyzer. It incorrectly assumed that switch labels cannot be inserted into random places inside of enclosed compound statements. It correctly assumes that you shouldn't be doing that. :) I propose making this change solely for aesthetics:

static void
pp_cxx_decl_specifier_seq (cxx_pretty_printer *pp, tree t)
{
  switch (TREE_CODE (t))
    {
    case FUNCTION_DECL:
      /* Constructors don't have return types.  And conversion functions
	 do not have a type-specifier in their return types.  */
      if (DECL_CONSTRUCTOR_P (t) || DECL_CONV_FN_P (t))
	pp_cxx_function_specifier (pp, t);
      else if (DECL_NONSTATIC_MEMBER_FUNCTION_P (t))
	pp_cxx_decl_specifier_seq (pp, TREE_TYPE (TREE_TYPE (t)));
      else
	default:
      pp_c_declaration_specifiers (pp_c_base (pp), t);
      break;
    }
}

This is, indeed, legal, but just because it is legal doesn't make it clear.
Even if it were reasonable, wouldn't it be written thus?

static void
pp_cxx_decl_specifier_seq (cxx_pretty_printer *pp, tree t)
{
  switch (TREE_CODE (t))
    {
    case FUNCTION_DECL:
      /* Constructors don't have return types.  And conversion functions
	 do not have a type-specifier in their return types.  */
      if (DECL_CONSTRUCTOR_P (t) || DECL_CONV_FN_P (t))
	pp_cxx_function_specifier (pp, t);
      else if (DECL_NONSTATIC_MEMBER_FUNCTION_P (t))
	pp_cxx_decl_specifier_seq (pp, TREE_TYPE (TREE_TYPE (t)));
      else
    default:
        pp_c_declaration_specifiers (pp_c_base (pp), t);
      break;
    }
}

Though I think this would be better:

static void
pp_cxx_decl_specifier_seq (cxx_pretty_printer *pp, tree t)
{
  switch (TREE_CODE (t))
    {
    case FUNCTION_DECL:
      /* Constructors don't have return types.  And conversion functions
	 do not have a type-specifier in their return types.  */

      if (DECL_CONSTRUCTOR_P (t) || DECL_CONV_FN_P (t))
	pp_cxx_function_specifier (pp, t), break;

      if (DECL_NONSTATIC_MEMBER_FUNCTION_P (t))
	pp_cxx_decl_specifier_seq (pp, TREE_TYPE (TREE_TYPE (t))), break;
      /* FALLTHROUGH */

    default:
      pp_c_declaration_specifiers (pp_c_base (pp), t);
      break;
    }
}

It surely looks cleaner to my eyes......


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]