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

Reply via email to