[PATCH] Fix indentation (and clang warning) in c-lang.c

[PATCH] Fix indentation (and clang warning) in c-lang.c

[Simon Marchi]

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

Re: [PATCH] Fix indentation (and clang warning) in c-lang.c

[Simon Marchi]

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

Re: [PATCH] Fix indentation (and clang warning) in c-lang.c

[Pedro Alves]

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

Re: [PATCH] Fix indentation (and clang warning) in c-lang.c

[Simon Marchi]

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

Re: [PATCH] Fix indentation (and clang warning) in c-lang.c

[Pedro Alves]

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

Re: [PATCH] Fix indentation (and clang warning) in c-lang.c

[Simon Marchi]

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

Re: [PATCH] Fix indentation (and clang warning) in c-lang.c

[Simon Marchi]

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