Re: [PATCH v2 8/9] rebase -i: learn to abbreviate command names

[Junio C Hamano]

Liam Beguin  writes:

> Since this came up, would it be a good thing to add -Wignored-qualifiers
> to the DEVELOPER flags?

Quite frankly, I am not sure if catching that particular warning
violation buys us much. As a return value from a function is never
an lvalue, what triggers the warning may certainly be an indication
of a sloppy coding, but otherwise I do not see it as diagnosing a
potential error.  "The programmer thought that the returned value
will only be assigned to a const variable and will never be
modified, but the language does not guarantee such a behaviour out
of the caller"---does such an incorrect expectation lead to an error
in the codepath that involves such a function?

Re: [PATCH v2 8/9] rebase -i: learn to abbreviate command names

[Liam Beguin]

Hi Junio,


On 27 December 2017 at 20:15, Junio C Hamano  wrote:
> Duy Nguyen  writes:
>
>> On Mon, Dec 4, 2017 at 5:17 AM, Liam Beguin  wrote:
>>> +static const char command_to_char(const enum todo_command command)
>>> +{
>>> +   if (command < TODO_COMMENT && todo_command_info[command].c)
>>> +   return todo_command_info[command].c;
>>> +   return comment_line_char;
>>> +}
>>
>> CC sequencer.o
>> sequencer.c:798:19: error: type qualifiers ignored on function return
>> type [-Werror=ignored-qualifiers]
>>  static const char command_to_char(const enum todo_command command)
>>^
>>
>> Maybe drop the first const.
>
> Thanks.  This topic has been in 'next' for quite some time and I
> wanted to merge it down to 'master' soonish, so I've added the
> following before doing so.

Thanks for taking the time. I had prepared the patch but was waiting to get
home to send it.
Only comment I have, maybe s/sequencer.c/rebase -i/ in the subject line
so it matches with the rest.

Since this came up, would it be a good thing to add -Wignored-qualifiers
to the DEVELOPER flags?

>
> -- >8 --
> From: Junio C Hamano 
> Date: Wed, 27 Dec 2017 11:12:45 -0800
> Subject: [PATCH] sequencer.c: drop 'const' from function return type
>
> With -Werror=ignored-qualifiers, a function that claims to return
> "const char" gets this error:
>
> CC sequencer.o
> sequencer.c:798:19: error: type qualifiers ignored on function return
> type [-Werror=ignored-qualifiers]
>  static const char command_to_char(const enum todo_command command)
>^
>
> Reported-by: Nguyễn Thái Ngọc Duy 
> Signed-off-by: Junio C Hamano 
> ---
>  sequencer.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/sequencer.c b/sequencer.c
> index 115085d39c..2a407cbe54 100644
> --- a/sequencer.c
> +++ b/sequencer.c
> @@ -795,7 +795,7 @@ static const char *command_to_string(const enum 
> todo_command command)
> die("Unknown command: %d", command);
>  }
>
> -static const char command_to_char(const enum todo_command command)
> +static char command_to_char(const enum todo_command command)
>  {
> if (command < TODO_COMMENT && todo_command_info[command].c)
> return todo_command_info[command].c;
> --
> 2.15.1-597-g62d91a8972
>

Thanks,
Liam

Re: [PATCH v2 8/9] rebase -i: learn to abbreviate command names

[Junio C Hamano]

Duy Nguyen  writes:

> On Mon, Dec 4, 2017 at 5:17 AM, Liam Beguin  wrote:
>> +static const char command_to_char(const enum todo_command command)
>> +{
>> +   if (command < TODO_COMMENT && todo_command_info[command].c)
>> +   return todo_command_info[command].c;
>> +   return comment_line_char;
>> +}
>
> CC sequencer.o
> sequencer.c:798:19: error: type qualifiers ignored on function return
> type [-Werror=ignored-qualifiers]
>  static const char command_to_char(const enum todo_command command)
>^
>
> Maybe drop the first const.

Thanks.  This topic has been in 'next' for quite some time and I
wanted to merge it down to 'master' soonish, so I've added the
following before doing so.

-- >8 --
From: Junio C Hamano 
Date: Wed, 27 Dec 2017 11:12:45 -0800
Subject: [PATCH] sequencer.c: drop 'const' from function return type

With -Werror=ignored-qualifiers, a function that claims to return
"const char" gets this error:

CC sequencer.o
sequencer.c:798:19: error: type qualifiers ignored on function return
type [-Werror=ignored-qualifiers]
 static const char command_to_char(const enum todo_command command)
   ^

Reported-by: Nguyễn Thái Ngọc Duy 
Signed-off-by: Junio C Hamano 
---
 sequencer.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/sequencer.c b/sequencer.c
index 115085d39c..2a407cbe54 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -795,7 +795,7 @@ static const char *command_to_string(const enum 
todo_command command)
die("Unknown command: %d", command);
 }
 
-static const char command_to_char(const enum todo_command command)
+static char command_to_char(const enum todo_command command)
 {
if (command < TODO_COMMENT && todo_command_info[command].c)
return todo_command_info[command].c;
-- 
2.15.1-597-g62d91a8972

Re: [PATCH v2 8/9] rebase -i: learn to abbreviate command names

[Duy Nguyen]

On Mon, Dec 25, 2017 at 10:39 PM, Liam Beguin  wrote:
> I'm curious, how did you build to get this error to show?
> I tried with the DEVELOPER 'flag' but nothing showed and -Wextra gave
> way too much messages...
> Did you just add -Wignored-qualifiers to CFLAGS?

I have a custom CFLAGS, created before DEVELOPER flag was added, which
is -Wextra -Werror plus about 5  -Wno-xxx to shut gcc up.
-- 
Duy

Re: [PATCH v2 8/9] rebase -i: learn to abbreviate command names

[Liam Beguin]

Hi Duy,

On Mon, 25 Dec 2017 at 07:48 Duy Nguyen  wrote:
>
> On Mon, Dec 4, 2017 at 5:17 AM, Liam Beguin  wrote:
> > +static const char command_to_char(const enum todo_command command)
> > +{
> > +   if (command < TODO_COMMENT && todo_command_info[command].c)
> > +   return todo_command_info[command].c;
> > +   return comment_line_char;
> > +}
>
> CC sequencer.o
> sequencer.c:798:19: error: type qualifiers ignored on function return
> type [-Werror=ignored-qualifiers]
>  static const char command_to_char(const enum todo_command command)
>^
>
> Maybe drop the first const.

Sorry, that's another copy-edit error I made that slipped through...
I'm curious, how did you build to get this error to show?
I tried with the DEVELOPER 'flag' but nothing showed and -Wextra gave
way too much messages...
Did you just add -Wignored-qualifiers to CFLAGS?

> --
> Duy

Thanks,
Liam

Re: [PATCH v2 8/9] rebase -i: learn to abbreviate command names

[Duy Nguyen]

On Mon, Dec 4, 2017 at 5:17 AM, Liam Beguin  wrote:
> +static const char command_to_char(const enum todo_command command)
> +{
> +   if (command < TODO_COMMENT && todo_command_info[command].c)
> +   return todo_command_info[command].c;
> +   return comment_line_char;
> +}

CC sequencer.o
sequencer.c:798:19: error: type qualifiers ignored on function return
type [-Werror=ignored-qualifiers]
 static const char command_to_char(const enum todo_command command)
   ^

Maybe drop the first const.
-- 
Duy