On 11/19/2015 01:50 AM, Joseph Myers wrote:
I don't think all the reformattings here are things we want to do globally for most source files.
While I do appreciate the sentiment behind the patch, I agree with all of Joseph's points. Especially the clearly incorrect changes should be reverted if the patch has already been applied.
@@ -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.
These ones (and any others like them) I would also like to see reverted. Both of them are contrary to existing practice.
Bernd
