Message ID | 20191216233637.29925-1-simon.marchi@efficios.com |
---|---|
State | New, archived |
Headers |
Received: (qmail 1701 invoked by alias); 16 Dec 2019 23:36:50 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: <gdb-patches.sourceware.org> List-Unsubscribe: <mailto:gdb-patches-unsubscribe-##L=##H@sourceware.org> List-Subscribe: <mailto:gdb-patches-subscribe@sourceware.org> List-Archive: <http://sourceware.org/ml/gdb-patches/> List-Post: <mailto:gdb-patches@sourceware.org> List-Help: <mailto:gdb-patches-help@sourceware.org>, <http://sourceware.org/ml/#faqs> Sender: gdb-patches-owner@sourceware.org Delivered-To: mailing list gdb-patches@sourceware.org Received: (qmail 1631 invoked by uid 89); 16 Dec 2019 23:36:50 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-22.6 required=5.0 tests=AWL, BAYES_00, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, RCVD_IN_DNSWL_NONE, SPF_PASS autolearn=ham version=3.3.1 spammy=shot, HDKIM-Filter:v2.10.3, HX-Languages-Length:2178, ling X-HELO: mail.efficios.com Received: from mail.efficios.com (HELO mail.efficios.com) (167.114.142.138) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Mon, 16 Dec 2019 23:36:46 +0000 Received: from localhost (ip6-localhost [IPv6:::1]) by mail.efficios.com (Postfix) with ESMTP id D11BC68B71E for <gdb-patches@sourceware.org>; Mon, 16 Dec 2019 18:36:44 -0500 (EST) Received: from mail.efficios.com ([IPv6:::1]) by localhost (mail02.efficios.com [IPv6:::1]) (amavisd-new, port 10032) with ESMTP id X3EPnZeUUds8; Mon, 16 Dec 2019 18:36:44 -0500 (EST) Received: from localhost (ip6-localhost [IPv6:::1]) by mail.efficios.com (Postfix) with ESMTP id 7AF5868B71A; Mon, 16 Dec 2019 18:36:44 -0500 (EST) DKIM-Filter: OpenDKIM Filter v2.10.3 mail.efficios.com 7AF5868B71A DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=efficios.com; s=default; t=1576539404; bh=2TeonSTdXEAP+EgzZG5cTd7ErgwVE9hdOKXmwUi4lFE=; h=From:To:Date:Message-Id:MIME-Version; b=eP27P5U8GFY4bG7fP4fbGkI14hH7K+qtle1GXwYuxSUeMQHfUolA27Vonr23vglcY l1iBTpki4YkXgPIAMB6y67VSWUoWNYD/79bvngPpCd895EnPPoOJ/LyhbzK5jYAXrY KG1CO0dew60PL1txWruDEblN7/3B5vHciu9+pOU9tqJo/YyEuTZlpBpeNOvT9O+UTB zuYVE+GrIumc5X2POXMd5G4y3nRmd+gnZ9Lfb7BukraYg9q5v62e87SH2cWL6wrorS fFsJt0BzlpJFRaMTBh/syxYRpCYrLoWfqa/eUE6Al4la8nFYuhVgeRfWhNGOB4Q2cu 0q2eqDsoR8LuQ== Received: from mail.efficios.com ([IPv6:::1]) by localhost (mail02.efficios.com [IPv6:::1]) (amavisd-new, port 10026) with ESMTP id dOOiqbHW-6V5; Mon, 16 Dec 2019 18:36:44 -0500 (EST) Received: from smarchi-efficios.internal.efficios.com (192-222-181-218.qc.cable.ebox.net [192.222.181.218]) by mail.efficios.com (Postfix) with ESMTPSA id 5A4EF68B714; Mon, 16 Dec 2019 18:36:44 -0500 (EST) From: Simon Marchi <simon.marchi@efficios.com> To: gdb-patches@sourceware.org Cc: Simon Marchi <simon.marchi@efficios.com> Subject: [PATCH] Fix indentation (and clang warning) in c-lang.c Date: Mon, 16 Dec 2019 18:36:37 -0500 Message-Id: <20191216233637.29925-1-simon.marchi@efficios.com> MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable |
Commit Message
Simon Marchi
Dec. 16, 2019, 11:36 p.m. UTC
I see this warning when building with clang: CXX c-lang.o /home/smarchi/src/binutils-gdb/gdb/c-lang.c:314:7: error: misleading indentation; statement is not part of the previous 'if' [-Werror,-Wmisleading-indentation] *length = i * width; ^ /home/smarchi/src/binutils-gdb/gdb/c-lang.c:308:4: note: previous statement is here if (extract_unsigned_integer (contents + i * width, ^ It took me a while to notice that some lines in that area have a spurious space before the tabs, at the beginning of the ling. I'm not sure how clang translates that to misleading indentation, but making the indentation correct gets rid of the error. There are many more instances of this in the code base (`grep -P '^ \t' *.c`), if others think it's a good idea, it would be pretty easy to fix them all up in one shot. gdb/ChangeLog: * c-lang.c (c_get_string, asm_language_defn): Remove space before tab. --- gdb/c-lang.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-)
Comments
On 2019-12-16 6:36 p.m., Simon Marchi wrote:
> I see this warning when building with clang:
I meant to add the clang version to the commit message, but forgot, I'll
add it locally. The version is (daily build from apt.llvm.org):
$ clang++-10 --version
clang version 10.0.0-+20191211091425+f99297176cd-1~exp1~20191211082036.1372
Simon
On 12/16/19 11:50 PM, Simon Marchi wrote: > On 2019-12-16 6:36 p.m., Simon Marchi wrote: >> I see this warning when building with clang: > > I meant to add the clang version to the commit message, but forgot, I'll > add it locally. The version is (daily build from apt.llvm.org): > > $ clang++-10 --version > clang version 10.0.0-+20191211091425+f99297176cd-1~exp1~20191211082036.1372 Curious. Are you saying that that changed recently in clang? I wonder whether clang's warning catches this case on purpose, or whether it's a regression. I asked David Malcolm, who added that warning to GCC, and he said that GCC's implementation respects tab stops, so small numbers of leading spaces before a tab are effectively ignored. Thanks, Pedro Alves
On 2019-12-17 2:24 p.m., Pedro Alves wrote: > Curious. Are you saying that that changed recently in clang? I wonder > whether clang's warning catches this case on purpose, or whether it's > a regression. I asked David Malcolm, who added that warning to GCC, and he > said that GCC's implementation respects tab stops, so small numbers of > leading spaces before a tab are effectively ignored. The warning appears to be new in clang 10, because it doesn't exist in clang 9: $ clang++ test.cpp -Wmisleading-indentation warning: unknown warning option '-Wmisleading-indentation'; did you mean '-Wbinding-in-condition'? [-Wunknown-warning-option] I can't tell if it's on purpose or not. I wonder if it's worth opening a bug on clang's bugzilla to ask whether it's intentional or not, it might help them tweak the warning before it's officially released... Simon
On 12/17/19 7:39 PM, Simon Marchi wrote: > On 2019-12-17 2:24 p.m., Pedro Alves wrote: >> Curious. Are you saying that that changed recently in clang? I wonder >> whether clang's warning catches this case on purpose, or whether it's >> a regression. I asked David Malcolm, who added that warning to GCC, and he >> said that GCC's implementation respects tab stops, so small numbers of >> leading spaces before a tab are effectively ignored. > > The warning appears to be new in clang 10, because it doesn't exist in clang 9: > > $ clang++ test.cpp -Wmisleading-indentation > warning: unknown warning option '-Wmisleading-indentation'; did you mean '-Wbinding-in-condition'? [-Wunknown-warning-option] > > I can't tell if it's on purpose or not. I wonder if it's worth opening a bug > on clang's bugzilla to ask whether it's intentional or not, it might help them > tweak the warning before it's officially released... Yes, I think it's worth it. Part of the point of using pre release compilers is to catch&fix issues before they are released. Thanks, Pedro Alves
On 2019-12-17 2:41 p.m., Pedro Alves wrote: > On 12/17/19 7:39 PM, Simon Marchi wrote: >> On 2019-12-17 2:24 p.m., Pedro Alves wrote: >>> Curious. Are you saying that that changed recently in clang? I wonder >>> whether clang's warning catches this case on purpose, or whether it's >>> a regression. I asked David Malcolm, who added that warning to GCC, and he >>> said that GCC's implementation respects tab stops, so small numbers of >>> leading spaces before a tab are effectively ignored. >> >> The warning appears to be new in clang 10, because it doesn't exist in clang 9: >> >> $ clang++ test.cpp -Wmisleading-indentation >> warning: unknown warning option '-Wmisleading-indentation'; did you mean '-Wbinding-in-condition'? [-Wunknown-warning-option] >> >> I can't tell if it's on purpose or not. I wonder if it's worth opening a bug >> on clang's bugzilla to ask whether it's intentional or not, it might help them >> tweak the warning before it's officially released... > > Yes, I think it's worth it. Part of the point of using pre release compilers > is to catch&fix issues before they are released. Ok, I opened this: https://bugs.llvm.org/show_bug.cgi?id=44324 Simon
On 2019-12-16 6:36 p.m., Simon Marchi wrote: > I see this warning when building with clang: > > CXX c-lang.o > /home/smarchi/src/binutils-gdb/gdb/c-lang.c:314:7: error: misleading indentation; statement is not part of the previous 'if' [-Werror,-Wmisleading-indentation] > *length = i * width; > ^ > /home/smarchi/src/binutils-gdb/gdb/c-lang.c:308:4: note: previous statement is here > if (extract_unsigned_integer (contents + i * width, > ^ > > It took me a while to notice that some lines in that area have a > spurious space before the tabs, at the beginning of the ling. I'm not > sure how clang translates that to misleading indentation, but making the > indentation correct gets rid of the error. > > There are many more instances of this in the code base (`grep -P '^ \t' > *.c`), if others think it's a good idea, it would be pretty easy to fix > them all up in one shot. > > gdb/ChangeLog: > > * c-lang.c (c_get_string, asm_language_defn): Remove space > before tab. > --- > gdb/c-lang.c | 10 +++++----- > 1 file changed, 5 insertions(+), 5 deletions(-) > > diff --git a/gdb/c-lang.c b/gdb/c-lang.c > index 846970af7b92..1f40e885d99a 100644 > --- a/gdb/c-lang.c > +++ b/gdb/c-lang.c > @@ -303,14 +303,14 @@ c_get_string (struct value *value, gdb::unique_xmalloc_ptr<gdb_byte> *buffer, > if (*length >= 0) > i = *length; > else > - /* Otherwise, look for a null character. */ > - for (i = 0; i < fetchlimit; i++) > + /* Otherwise, look for a null character. */ > + for (i = 0; i < fetchlimit; i++) > if (extract_unsigned_integer (contents + i * width, > width, byte_order) == 0) > - break; > + break; > > /* I is now either a user-defined length, the number of non-null > - characters, or FETCHLIMIT. */ > + characters, or FETCHLIMIT. */ > *length = i * width; > buffer->reset ((gdb_byte *) xmalloc (*length)); > memcpy (buffer->get (), contents, *length); > @@ -1119,7 +1119,7 @@ extern const struct language_defn asm_language_defn = > 0, /* String lower bound */ > default_word_break_characters, > default_collect_symbol_completion_matches, > - c_language_arch_info, /* FIXME: la_language_arch_info. */ > + c_language_arch_info, /* FIXME: la_language_arch_info. */ > default_print_array_index, > default_pass_by_reference, > c_watch_location_expression, > -- > 2.24.1 > I pushed this one. Simon
diff --git a/gdb/c-lang.c b/gdb/c-lang.c index 846970af7b92..1f40e885d99a 100644 --- a/gdb/c-lang.c +++ b/gdb/c-lang.c @@ -303,14 +303,14 @@ c_get_string (struct value *value, gdb::unique_xmalloc_ptr<gdb_byte> *buffer, if (*length >= 0) i = *length; else - /* Otherwise, look for a null character. */ - for (i = 0; i < fetchlimit; i++) + /* Otherwise, look for a null character. */ + for (i = 0; i < fetchlimit; i++) if (extract_unsigned_integer (contents + i * width, width, byte_order) == 0) - break; + break; /* I is now either a user-defined length, the number of non-null - characters, or FETCHLIMIT. */ + characters, or FETCHLIMIT. */ *length = i * width; buffer->reset ((gdb_byte *) xmalloc (*length)); memcpy (buffer->get (), contents, *length); @@ -1119,7 +1119,7 @@ extern const struct language_defn asm_language_defn = 0, /* String lower bound */ default_word_break_characters, default_collect_symbol_completion_matches, - c_language_arch_info, /* FIXME: la_language_arch_info. */ + c_language_arch_info, /* FIXME: la_language_arch_info. */ default_print_array_index, default_pass_by_reference, c_watch_location_expression,