Re: [PATCH 1/4] -Wmisleading-indentation: don't warn in presence of entirely blank lines

[Jeff Law <law@redhat.com>]

On 10/29/2015 10:49 AM, David Malcolm wrote:
> Attempting to bootstrap gcc with -Wmisleading-indentation enabled I ran
> into a few failures where the indentation, although bad, was arguably
> not misleading.
>
> In regrename.c:scan_rtx_address:
>
>    1308      case PRE_MODIFY:
>    1309        /* If the target doesn't claim to handle autoinc, this must be
>    1310           something special, like a stack push.  Kill this chain.  */
>    1311      if (!AUTO_INC_DEC)
>    1312        action = mark_all_read;
>    1313
>    1314        break;
>                ^ this is indented at the same level as the "action =" code,
>                but clearly isn't guarded by the if () at line 1311.
>
> In gcc/fortran/io.c:gfc_match_open:
>
>    1997          {
>    1998            static const char *delim[] = { "APOSTROPHE", "QUOTE", "NONE", NULL };
>    1999
>    2000          if (!is_char_type ("DELIM", open->delim))
>    2001            goto cleanup;
>    2002
>    2003            if (!compare_to_allowed_values ("DELIM", delim, NULL, NULL,
>    2004                                            open->delim->value.character.string,
>    2005                                            "OPEN", warn))
>                    ^ this is indented with the "goto cleanup;" due to
>                      lines 2000-2001 not being indented enough, but
>                      line 2003 clearly isn't guarded by the
>                      "if (!is_char_type" conditional.
>
> In gcc/function.c:locate_and_pad_parm:
>
>    4118        locate->slot_offset.constant = -initial_offset_ptr->constant;
>    4119        if (initial_offset_ptr->var)
>    4120          locate->slot_offset.var = size_binop (MINUS_EXPR, ssize_int (0),
>    4121                                                initial_offset_ptr->var);
>    4122
>    4123          {
>    4124            tree s2 = sizetree;
>    4125            if (where_pad != none
>    4126                && (!tree_fits_uhwi_p (sizetree)
>    4127                    || (tree_to_uhwi (sizetree) * BITS_PER_UNIT) % round_boundary))
>    4128              s2 = round_up (s2, round_boundary / BITS_PER_UNIT);
>    4129            SUB_PARM_SIZE (locate->slot_offset, s2);
>    4130          }
>                  ^ this block is not guarded by the
>                    "if (initial_offset_ptr->var)"
>                    and the whitespace line (4122) is likely to make a
>                    human reader of the code read it that way also.
>
> In each case, a blank line separated the guarded code from followup code
> that wasn't guarded, and to my eyes, the blank line makes the meaning of
> the badly-indented code sufficiently clear that it seems unjustified to
> issue a -Wmisleading-indentation warning.
I would argue that each of these does represent misleading indentation 
and that the warning is warranted for each.  Perhaps they aren't as bad 
as prior cases, but I'd still consider them mis-leading.

Even more so if you have to work in codebases with differing styles 
(particularly where the open curly goes).

Jeff