On Mon, 2 Nov 2015, David Malcolm wrote:
On Sun, 2015-11-01 at 17:06 -0500, Patrick Palka wrote:
On Fri, Oct 30, 2015 at 5:03 AM, Richard Biener
<richard.guent...@gmail.com> wrote:
On Thu, Oct 29, 2015 at 6:38 PM, Jeff Law <l...@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®rtested 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
I don't have build logs but I have diffs that indicates where in the
code the warnings appear and how to address the warnings (roughly).
For binutils-gdb:
diff --git a/bfd/coff-i386.c b/bfd/coff-i386.c
index a9725c4..fca7887 100644
--- a/bfd/coff-i386.c
+++ b/bfd/coff-i386.c
@@ -139,7 +139,7 @@ coff_i386_reloc (bfd *abfd,
#define DOIT(x) \
x = ((x & ~howto->dst_mask) | (((x & howto->src_mask) + diff) &
howto->dst_mask))
- if (diff != 0)
+ if (diff != 0)
{
reloc_howto_type *howto = reloc_entry->howto;
unsigned char *addr = (unsigned char *) data + reloc_entry->address;
diff --git a/bfd/coff-x86_64.c b/bfd/coff-x86_64.c
index 4e6420a..23ffecb 100644
--- a/bfd/coff-x86_64.c
+++ b/bfd/coff-x86_64.c
@@ -138,7 +138,7 @@ coff_amd64_reloc (bfd *abfd,
#define DOIT(x) \
x = ((x & ~howto->dst_mask) | (((x & howto->src_mask) + diff) &
howto->dst_mask))
- if (diff != 0)
+ if (diff != 0)
{
reloc_howto_type *howto = reloc_entry->howto;
unsigned char *addr = (unsigned char *) data + reloc_entry->address;
diff --git a/gdb/ada-lang.c b/gdb/ada-lang.c
index fff4862..2559a36 100644
--- a/gdb/ada-lang.c
+++ b/gdb/ada-lang.c
@@ -11359,9 +11359,11 @@ ada_evaluate_subexp (struct type *expect_type, struct
expression *exp,
return value_zero (ada_aligned_type (type), lval_memory);
}
else
- arg1 = ada_value_struct_elt (arg1, &exp->elts[pc + 2].string, 0);
- arg1 = unwrap_value (arg1);
- return ada_to_fixed_value (arg1);
+ {
+ arg1 = ada_value_struct_elt (arg1, &exp->elts[pc + 2].string, 0);
+ arg1 = unwrap_value (arg1);
+ return ada_to_fixed_value (arg1);
+ }
case OP_TYPE:
/* The value is not supposed to be used. This is here to make it
diff --git a/gdb/c-typeprint.c b/gdb/c-typeprint.c
index 1af477c..1bd3b12 100644
--- a/gdb/c-typeprint.c
+++ b/gdb/c-typeprint.c
@@ -1305,26 +1305,26 @@ c_type_print_base (struct type *type, struct ui_file
*stream,
if (TYPE_NFIELDS (type) != 0 || TYPE_NFN_FIELDS (type) != 0)
fprintf_filtered (stream, "\n");
- for (i = 0; i < TYPE_TYPEDEF_FIELD_COUNT (type); i++)
- {
- struct type *target = TYPE_TYPEDEF_FIELD_TYPE (type, i);
-
- /* Dereference the typedef declaration itself. */
- gdb_assert (TYPE_CODE (target) == TYPE_CODE_TYPEDEF);
- target = TYPE_TARGET_TYPE (target);
-
- print_spaces_filtered (level + 4, stream);
- fprintf_filtered (stream, "typedef ");
-
- /* We want to print typedefs with substitutions
- from the template parameters or globally-known
- typedefs but not local typedefs. */
- c_print_type (target,
- TYPE_TYPEDEF_FIELD_NAME (type, i),
- stream, show - 1, level + 4,
- &semi_local_flags);
- fprintf_filtered (stream, ";\n");
- }
+ for (i = 0; i < TYPE_TYPEDEF_FIELD_COUNT (type); i++)
+ {
+ struct type *target = TYPE_TYPEDEF_FIELD_TYPE (type, i);
+
+ /* Dereference the typedef declaration itself. */
+ gdb_assert (TYPE_CODE (target) == TYPE_CODE_TYPEDEF);
+ target = TYPE_TARGET_TYPE (target);
+
+ print_spaces_filtered (level + 4, stream);
+ fprintf_filtered (stream, "typedef ");
+
+ /* We want to print typedefs with substitutions
+ from the template parameters or globally-known
+ typedefs but not local typedefs. */
+ c_print_type (target,
+ TYPE_TYPEDEF_FIELD_NAME (type, i),
+ stream, show - 1, level + 4,
+ &semi_local_flags);
+ fprintf_filtered (stream, ";\n");
+ }
}
fprintfi_filtered (level, stream, "}");
diff --git a/gdb/inflow.c b/gdb/inflow.c
index d38a43d..87988ea 100644
--- a/gdb/inflow.c
+++ b/gdb/inflow.c
@@ -413,7 +413,7 @@ child_terminal_ours_1 (int output_only)
if (tinfo->run_terminal != NULL || gdb_has_a_terminal () == 0)
return;
- {
+ {
#ifdef SIGTTOU
/* Ignore this signal since it will happen when we try to set the
pgrp. */
@@ -497,7 +497,7 @@ child_terminal_ours_1 (int output_only)
result = fcntl (0, F_SETFL, our_terminal_info.tflags);
result = fcntl (0, F_SETFL, our_terminal_info.tflags);
#endif
- }
+ }
}
/* Per-inferior data key. */
diff --git a/gdb/linux-record.c b/gdb/linux-record.c
index 091ac8a..18f8fbf 100644
--- a/gdb/linux-record.c
+++ b/gdb/linux-record.c
@@ -112,7 +112,7 @@ record_linux_sockaddr (struct regcache *regcache,
"memory at addr = 0x%s len = %d.\n",
phex_nz (len, tdep->size_pointer),
tdep->size_int);
- return -1;
+ return -1;
}
addrlen = (int) extract_unsigned_integer (a, tdep->size_int, byte_order);
if (addrlen <= 0 || addrlen > tdep->size_sockaddr)
@@ -150,7 +150,7 @@ record_linux_msghdr (struct regcache *regcache,
"len = %d.\n",
phex_nz (addr, tdep->size_pointer),
tdep->size_msghdr);
- return -1;
+ return -1;
}
/* msg_name msg_namelen */
@@ -188,7 +188,7 @@ record_linux_msghdr (struct regcache *regcache,
"len = %d.\n",
phex_nz (addr,tdep->size_pointer),
tdep->size_iovec);
- return -1;
+ return -1;
}
tmpaddr = (CORE_ADDR) extract_unsigned_integer (iov,
tdep->size_pointer,
@@ -983,7 +983,7 @@ Do you want to stop the program?"),
"memory at addr = 0x%s len = %d.\n",
OUTPUT_REG (tmpulongest, tdep->arg2),
tdep->size_ulong);
- return -1;
+ return -1;
}
tmpulongest = extract_unsigned_integer (a, tdep->size_ulong,
byte_order);
====
And for glibc:
diff --git a/stdio-common/vfscanf.c b/stdio-common/vfscanf.c
index 1382eb5..1963c31 100644
--- a/stdio-common/vfscanf.c
+++ b/stdio-common/vfscanf.c
@@ -1711,7 +1711,7 @@ _IO_vfscanf_internal (_IO_FILE *s, const char *format,
_IO_va_list argptr,
/* The last thousands character will be added back by
the char_buffer_add below. */
- --charbuf.current;
+ --charbuf.current;
#endif
}
else
diff --git a/stdlib/strtol_l.c b/stdlib/strtol_l.c
index 8f6163d..0fb9a92 100644
--- a/stdlib/strtol_l.c
+++ b/stdlib/strtol_l.c
@@ -351,8 +351,8 @@ INTERNAL (__strtol_l) (const STRING_TYPE *nptr, STRING_TYPE
**endptr,
&& (wchar_t) c != thousands
# else
&& ({ for (cnt = 0; cnt < thousands_len; ++cnt)
- if (thousands[cnt] != end[cnt])
- break;
+ if (thousands[cnt] != end[cnt])
+ break;
cnt < thousands_len; })
# endif
&& (!ISALPHA (c)
diff --git a/sysdeps/ieee754/flt-32/k_rem_pio2f.c
b/sysdeps/ieee754/flt-32/k_rem_pio2f.c
index 0c7685c..a0d844c 100644
--- a/sysdeps/ieee754/flt-32/k_rem_pio2f.c
+++ b/sysdeps/ieee754/flt-32/k_rem_pio2f.c
@@ -65,7 +65,8 @@ int __kernel_rem_pio2f(float *x, float *y, int e0, int nx,
int prec, const int32
/* compute q[0],q[1],...q[jk] */
for (i=0;i<=jk;i++) {
- for(j=0,fw=0.0;j<=jx;j++) fw += x[j]*f[jx+i-j]; q[i] = fw;
+ for(j=0,fw=0.0;j<=jx;j++) fw += x[j]*f[jx+i-j];
+ q[i] = fw;
}
jz = jk;
