I don't think all the reformattings here are things we want to do globally for most source files. E.g.
> @@ -75,18 +74,17 @@ get_mask_first_set_bit (unsigned mask) > static bool > has_undefined_value_p (tree t) > { > - return (ssa_undefined_value_p (t) > - || (possibly_undefined_names > - && possibly_undefined_names->contains (t))); > + return ssa_undefined_value_p (t) > + || (possibly_undefined_names > + && possibly_undefined_names->contains (t)); This seems plain wrong. The GNU Coding Standards explicitly say that in such cases where a binary operator goes on the start of the next line, you should insert extra parentheses so that Emacs will get the indentation right (even though otherwise parentheses shouldn't be used with "return"). So the old code was better. > static void > -warn_uninit (enum opt_code wc, tree t, tree expr, tree var, > - const char *gmsgid, void *data, location_t phiarg_loc) > +warn_uninit (enum opt_code wc, tree t, tree expr, tree var, const char > *gmsgid, > + void *data, location_t phiarg_loc) Just because it's OK to go up to an 80- or 79-column limit if necessary doesn't mean code should be reformatted to do so. Breaking a bit before that is perfectly reasonable in general (and in some cases, the choice of where to break may be based on logical grouping of arguments). > @@ -135,10 +133,9 @@ warn_uninit (enum opt_code wc, tree t, tree expr, tree > var, > > /* TREE_NO_WARNING either means we already warned, or the front end > wishes to suppress the warning. */ > - if ((context > - && (gimple_no_warning_p (context) > - || (gimple_assign_single_p (context) > - && TREE_NO_WARNING (gimple_assign_rhs1 (context))))) > + if ((context && (gimple_no_warning_p (context) > + || (gimple_assign_single_p (context) > + && TREE_NO_WARNING (gimple_assign_rhs1 (context))))) I think in cases such as this, putting the operator && on the start of the next line to make subsequent lines less-indented makes sense. Again, the new formatting isn't incorrect, but that doesn't make it a desirable change. > @@ -217,24 +212,20 @@ warn_uninitialized_vars (bool > warn_possibly_uninitialized) > so must be limited which means we would miss warning > opportunities. */ > use = gimple_vuse (stmt); > - if (use > - && gimple_assign_single_p (stmt) > - && !gimple_vdef (stmt) > + if (use && gimple_assign_single_p (stmt) && !gimple_vdef (stmt) > && SSA_NAME_IS_DEFAULT_DEF (use)) I think there may be a preference, where it's necessary to use multiple lines (if the whole condition doesn't fit on one line) for a condition like this, for each "&& condition" to go on its own line, rather than some lines having multiple conditions. > /* Do not warn if it can be initialized outside this function. */ > - if (TREE_CODE (base) != VAR_DECL > - || DECL_HARD_REGISTER (base) > + if (TREE_CODE (base) != VAR_DECL || DECL_HARD_REGISTER (base) > || is_global_var (base)) Likewise. > @@ -2393,15 +2338,28 @@ class pass_late_warn_uninitialized : public > gimple_opt_pass > public: > pass_late_warn_uninitialized (gcc::context *ctxt) > : gimple_opt_pass (pass_data_late_warn_uninitialized, ctxt) > - {} > + { > + } Is that really what we want? I think those examples are sufficient to illustrate the problems with automatic conversion from one correct format to another correct format (as opposed to fixing cases where the formatting is unambiguously incorrect, which covers plenty of the changes in this patch). -- Joseph S. Myers jos...@codesourcery.com