[Bug c/66220] New: -Wmisleading-indentation false/inconsistent warning

[Bug c/66220] New: -Wmisleading-indentation false/inconsistent warning

[sirl at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=66220

            Bug ID: 66220
           Summary: -Wmisleading-indentation false/inconsistent warning
           Product: gcc
           Version: 6.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: c
          Assignee: unassigned at gcc dot gnu.org
          Reporter: sirl at gcc dot gnu.org
  Target Milestone: ---

Created attachment 35578
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=35578&action=edit
Testcase to reproduce

The following indenting style generates a false warning:

int test1(int v)
{
    int res = 28;

    if (v == 2)
    {
        res = 27;
    } else
    {
        res = 18;
    }
    return res;
}

test-indent.c: In function 'test1':
test-indent.c:13:5: warning: statement is indented as if it were guarded by...
[-Wmisleading-indentation]
     return res;
      ^
test-indent.c:9:7: note: ...this 'else' clause, but it is not
     } else
        ^

Even though I don't like this style, I don't think it's misleading.
If you change the 'else' to 'else if ()' the warning goes away, that's why
think it's at least inconsistent.

[Bug c/66220] -Wmisleading-indentation false/inconsistent warning

[mpolacek at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=66220

Marek Polacek <mpolacek at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |NEW
   Last reconfirmed|                            |2015-05-20
                 CC|                            |dmalcolm at gcc dot gnu.org,
                   |                            |mpolacek at gcc dot gnu.org
   Target Milestone|---                         |6.0
     Ever confirmed|0                           |1

--- Comment #1 from Marek Polacek <mpolacek at gcc dot gnu.org> ---
Confirmed.

[Bug c/66220] -Wmisleading-indentation false/inconsistent warning

[dmalcolm at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=66220

--- Comment #2 from David Malcolm <dmalcolm at gcc dot gnu.org> ---
Thanks.

I ran into a variant of this whilst testing -Wmisleading-indentation on the
linux kernel, where a preprocessor macro conditionalizes the "if/else"; here's
the test case I reduced it to:

/* This variant of K&R-style formatting (in the presence of conditional
   compilation) shouldn't lead to a warning.

   Based on false positive seen with r223098 when compiling
   linux-4.0.3:arch/x86/crypto/aesni-intel_glue.c:aesni_init.  */

void
fn_36 (void)
{
#if 1 /* e.g. some configuration variable.  */
        if (flagA) {
                foo(0);
                foo(1);
                foo(2);
        } else
#endif
        {
                foo(3);
                foo(4);
                foo(5);
        }
        foo(6); /* We shouldn't warn here.  */
}

I have a fix for this, by requiring that the visual column of the guard
("else") be <= that of the stmts, which works for all of the testcases
(including the new ones I posted as
https://gcc.gnu.org/ml/gcc-patches/2015-05/msg01846.html ), apart from fn_15:

#define FOR_EACH(VAR, START, STOP) for ((VAR) = (START); (VAR) < (STOP);
(VAR++)) /* { dg-message "36: ...this 'for' clause, but it is not" } */
void fn_15 (void)
{
  int i;
  FOR_EACH (i, 0, 10) /* { dg-message "3: in expansion of macro" } */
    foo (i);
    bar (i, i); /* { dg-warning "statement is indented as if it were guarded
by..." } */
}
#undef FOR_EACH

which then fails to report the warning due to it using the location of the
"for" in the defn of macro FOR_EACH.

Fixing that will require some reworking on how we handle macro expansions.

[Bug c/66220] -Wmisleading-indentation false/inconsistent warning

[dmalcolm at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=66220

David Malcolm <dmalcolm at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Assignee|unassigned at gcc dot gnu.org      |dmalcolm at gcc dot gnu.org

--- Comment #3 from David Malcolm <dmalcolm at gcc dot gnu.org> ---
Patch posted as:
https://gcc.gnu.org/ml/gcc-patches/2015-05/msg01858.html

[Bug c/66220] -Wmisleading-indentation false/inconsistent warning

[sirl at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=66220

--- Comment #4 from Franz Sirl <sirl at gcc dot gnu.org> ---
Patch from #c3 works fine for our codebase, I couldn't spot any false positives
anymore.

[Bug c/66220] -Wmisleading-indentation false/inconsistent warning

[dmalcolm at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=66220

--- Comment #5 from David Malcolm <dmalcolm at gcc dot gnu.org> ---
Author: dmalcolm
Date: Tue Jun  2 18:45:50 2015
New Revision: 224041

URL: https://gcc.gnu.org/viewcvs?rev=224041&root=gcc&view=rev
Log:
PR c/66220: Fix false positive from -Wmisleading-indentation

gcc/c-family/ChangeLog:
        PR c/66220:
        * c-indentation.c (should_warn_for_misleading_indentation): Use
        expand_location rather than expand_location_to_spelling_point.
        Don't warn if the guarding statement is more indented than the
        next/body stmts.

gcc/testsuite/ChangeLog:
        PR c/66220:
        * c-c++-common/Wmisleading-indentation.c (fn_35): New.
        (fn_36): New.


Modified:
    trunk/gcc/c-family/ChangeLog
    trunk/gcc/c-family/c-indentation.c
    trunk/gcc/testsuite/ChangeLog
    trunk/gcc/testsuite/c-c++-common/Wmisleading-indentation.c

[Bug c/66220] -Wmisleading-indentation false/inconsistent warning

[dmalcolm at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=66220

David Malcolm <dmalcolm at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |RESOLVED
         Resolution|---                         |FIXED

--- Comment #6 from David Malcolm <dmalcolm at gcc dot gnu.org> ---
(In reply to Franz Sirl from comment #4)
> Patch from #c3 works fine for our codebase, I couldn't spot any false
> positives anymore.

Thanks.

Should be fixed as of r224041.