This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH 4/4] Add -Wmisleading-indentation to -Wall


On Sun, 2015-11-01 at 17:06 -0500, Patrick Palka wrote:
> On Fri, Oct 30, 2015 at 5:03 AM, Richard Biener
> <richard.guenther@gmail.com> wrote:
> > On Thu, Oct 29, 2015 at 6:38 PM, Jeff Law <law@redhat.com> wrote:
> >> On 10/29/2015 10:49 AM, David Malcolm wrote:
> >>>
> >>> Our documentation describes -Wall as enabling "all the warnings about
> >>> constructions that some users consider questionable, and that are easy to
> >>> avoid
> >>> (or modify to prevent the warning), even in conjunction with macros."
> >>>
> >>> I believe that -Wmisleading-indentation meets these criteria, and is
> >>> likely to be of benefit to users who may not read release notes; it
> >>> warns for indentation that's misleading, but not for indentation
> >>> that's merely bad: the former are places where a user will likely
> >>> want to fix the code.
> >>>
> >>> The fix is usually easy and obvious: fix the misleadingly-indented
> >>> code.  If that isn't an option for some reason, pragmas can be used to
> >>> turn off the warning for a particular fragment of code:
> >>>
> >>>    #pragma GCC diagnostic push
> >>>    #pragma GCC diagnostic ignored "-Wmisleading-indentation"
> >>>      if (flag)
> >>>        x = 3;
> >>>        y = 2;
> >>>    #pragma GCC diagnostic pop
> >>>
> >>> -Wmisleading-indentation has been tested with a variety of indentation
> >>> styles (see gcc/testsuite/c-c++-common/Wmisleading-indentation.c)
> >>> and on a variety of real-world projects.  For example, in:
> >>>    https://www.mail-archive.com/gcc-patches@gcc.gnu.org/msg119790.html
> >>> Patrick reports:
> >>> "Tested by building the linux, git, vim, sqlite and gdb-binutils sources
> >>>   with -Wmisleading-indentation."
> >>>
> >>> With the tweak earlier in this kit I believe we now have a good
> >>> enough signal:noise ratio for this warning to be widely used; hence this
> >>> patch adds the warning to -Wall.
> >>>
> >>> Bootstrapped&regrtested with x86_64-pc-linux-gnu.
> >>>
> >>> OK for trunk?
> >>>
> >>> gcc/c-family/ChangeLog:
> >>>         * c.opt (Wmisleading-indentation): Add to -Wall for C and C++.
> >>>
> >>> gcc/ChangeLog:
> >>>         * doc/invoke.texi (-Wall): Add -Wmisleading-indentation to the
> >>>         list.
> >>>         (-Wmisleading-indentation): Update documentation to reflect
> >>>         being enabled by -Wall in C/C++.
> >>
> >> I'm sure we'll get some grief for this :-)
> >>
> >> Approved once we're clean in GCC.  I'm going to explicitly say that we'll
> >> have to watch for fallout, particularly as we start getting feedback from
> >> Debian & Fedora mass-rebuilds as we approach release time.  If the fallout
> >> is too bad, we'll have to reconsider.
> >>
> >> I'll pre-approve patches which fix anything caught by this option in GCC as
> >> long as the fix just adjusts whitespace :-)
> >
> > Please at least check also binutils and gdb and other packages that use -Werror
> > (well, just rebuild Fedora world).
> 
> FYI binutils, gdb and glibc, from git, all fail to build due to
> -Wmisleading-indentation warnings/errors. (None of the warnings are
> bogus (IMO), though I don't think any of the warnings expose a real
> bug either.)

Bother.   Do you happen to have the logs handy? (or could you upload the
somewhere?)

I tried building binutils+gdb 854eb72b00ba46d65ce36dc3432f01e223ce44cb
with gcc6+this kit (on x86_64) but I get:
In file included from ../../src/bfd/archive.c:143:0:
../../src/bfd/../include/bfdlink.h:452:38: error: field âcompress_debugâ
has incomplete type
   enum compressed_debug_section_type compress_debug;
                                      ^
../../src/bfd/archive.c: In function âopen_nested_fileâ:
../../src/bfd/archive.c:393:12: error: âbfd {aka struct bfd}â has no
member named âlto_outputâ
       n_bfd->lto_output = archive->lto_output;
            ^
which appears to be unrelated snafu from the binutils+gdb side.

Thanks
Dave


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]