'warning: unused parameter'

[Stephen Leake-3]

I've succeeded in setting up a MinGW64 environment. It's quite nice once
you get past the tools install issues. I have not committed
INSTALL_windows_mingw64.txt yet; soon.

I'm getting lots of 'warning: unused parameter' messages from g++:

../monotone/src/simplestring_xform.hh:51:61: warning: unused parameter 'thing' [-Wunused-parameter]
 origin::type get_made_from<std::string>(std::string const & thing)
                                                             ^

The code that generates this is in simplestring_xform.hh:

template<> inline
origin::type get_made_from<std::string>(std::string const & thing)
{
  return origin::internal;
}

Is there a way to mark 'thing' as unused? or do we have to disable that
warning with -Wno-unused-parameter?

--
-- Stephe

_______________________________________________
Monotone-devel mailing list
[hidden email]
https://lists.nongnu.org/mailman/listinfo/monotone-devel

[Stephen Leake-3]

Stephen Leake <[hidden email]> writes:

> The code that generates this is in simplestring_xform.hh:
>
> template<> inline
> origin::type get_made_from<std::string>(std::string const & thing)
> {
>   return origin::internal;
> }
>
> Is there a way to mark 'thing' as unused? or do we have to disable that
> warning with -Wno-unused-parameter?

I found three answers on stack overflow:

1) comment out or delete the parameter name:

    a) origin::type get_made_from<std::string>(std::string const & /* thing */)

    b) origin::type get_made_from<std::string>(std::string const & )

2) cast it to void:

    (void)thing;

I prefer 1a; it most clearly documents that we know there is a parameter
by that name, but we are not using it in this case. Any objections?

--
-- Stephe

_______________________________________________
Monotone-devel mailing list
[hidden email]
https://lists.nongnu.org/mailman/listinfo/monotone-devel

[Stephen Leake-3]

Stephen Leake <[hidden email]> writes:

> Stephen Leake <[hidden email]> writes:
>
>> The code that generates this is in simplestring_xform.hh:
>>
>> template<> inline
>> origin::type get_made_from<std::string>(std::string const & thing)
>> {
>>   return origin::internal;
>> }
>>
>> Is there a way to mark 'thing' as unused? or do we have to disable that
>> warning with -Wno-unused-parameter?
>
> I found three answers on stack overflow:
>
> 1) comment out or delete the parameter name:
>
>     a) origin::type get_made_from<std::string>(std::string const & /* thing */)
>
>     b) origin::type get_made_from<std::string>(std::string const & )
>
> 2) cast it to void:
>
>     (void)thing;
>
> I prefer 1a; it most clearly documents that we know there is a parameter
> by that name, but we are not using it in this case. Any objections?

However, that is not appropriate in this case (command.hh):

#define CMD_NO_WORKSPACE(C, name, aliases, parent, params, abstract, \
                         desc, opts)                                 \
namespace commands {                                                 \
  class cmd_ ## C : public command                                   \
  {                                                                  \
  public:                                                            \
    cmd_ ## C() : command(name, aliases, parent, false, false,       \
                          params, abstract, desc, false,             \
                          options::options_type() | opts, true)      \
    {}                                                               \
    virtual void exec(app_state & app,                               \
                      command_id const & execid,                     \
                      args_vector const & args) const;               \
  };                                                                 \
  cmd_ ## C C ## _cmd;                                               \
}                                                                    \
void commands::cmd_ ## C::exec(app_state & app,                      \
                               command_id const & execid,            \
                               args_vector const & args) const


In most uses of CMD_NO_WORKSPACE, "execid" is not used, but it is used
in some (cmd_netsync.cc clone).

So we have to use (void)exec_id in the body of each case where it is not
used.

--
-- Stephe

_______________________________________________
Monotone-devel mailing list
[hidden email]
https://lists.nongnu.org/mailman/listinfo/monotone-devel

[Václav Zeman]

On 30 April 2014 11:05, Stephen Leake wrote:

>
> Stephen Leake <[hidden email]> writes:
>
> > Stephen Leake <[hidden email]> writes:
> >
> >> The code that generates this is in simplestring_xform.hh:
> >>
> >> template<> inline
> >> origin::type get_made_from<std::string>(std::string const & thing)
> >> {
> >>   return origin::internal;
> >> }
> >>
> >> Is there a way to mark 'thing' as unused? or do we have to disable that
> >> warning with -Wno-unused-parameter?
> >
> > I found three answers on stack overflow:
> >
> > 1) comment out or delete the parameter name:
> >
> >     a) origin::type get_made_from<std::string>(std::string const & /* thing */)
> >
> >     b) origin::type get_made_from<std::string>(std::string const & )
> >
> > 2) cast it to void:
> >
> >     (void)thing;
> >
> > I prefer 1a; it most clearly documents that we know there is a parameter
> > by that name, but we are not using it in this case. Any objections?
>
> However, that is not appropriate in this case (command.hh):
>
> #define CMD_NO_WORKSPACE(C, name, aliases, parent, params, abstract, \
>                          desc, opts)                                 \
> namespace commands {                                                 \
>   class cmd_ ## C : public command                                   \
>   {                                                                  \
>   public:                                                            \
>     cmd_ ## C() : command(name, aliases, parent, false, false,       \
>                           params, abstract, desc, false,             \
>                           options::options_type() | opts, true)      \
>     {}                                                               \
>     virtual void exec(app_state & app,                               \
>                       command_id const & execid,                     \
>                       args_vector const & args) const;               \
>   };                                                                 \
>   cmd_ ## C C ## _cmd;                                               \
> }                                                                    \
> void commands::cmd_ ## C::exec(app_state & app,                      \
>                                command_id const & execid,            \
>                                args_vector const & args) const
>
>
> In most uses of CMD_NO_WORKSPACE, "execid" is not used, but it is used
> in some (cmd_netsync.cc clone).
>
> So we have to use (void)exec_id in the body of each case where it is not
> used.

You could special case GCC and introduce your own macro that expands
to the "unused" attribute in case of GCC:
http://gcc.gnu.org/onlinedocs/gcc-4.9.0/gcc/Function-Attributes.html#index-g_t_0040code_007bunused_007d-attribute_002e-3000

--
VZ

_______________________________________________
Monotone-devel mailing list
[hidden email]
https://lists.nongnu.org/mailman/listinfo/monotone-devel

[Stephen Leake-3]

Václav Zeman <[hidden email]> writes:

> You could special case GCC and introduce your own macro that expands
> to the "unused" attribute in case of GCC:
> http://gcc.gnu.org/onlinedocs/gcc-4.9.0/gcc/Function-Attributes.html#index-g_t_0040code_007bunused_007d-attribute_002e-3000

that page says it is for functions, not function arguments.

--
-- Stephe

_______________________________________________
Monotone-devel mailing list
[hidden email]
https://lists.nongnu.org/mailman/listinfo/monotone-devel

[Markus Wanner-2]

On 04/30/2014 10:40 AM, Stephen Leake wrote:
> I've succeeded in setting up a MinGW64 environment. It's quite nice once
> you get past the tools install issues. I have not committed
> INSTALL_windows_mingw64.txt yet; soon.

Is is really that much different from mingw32 that it warrants a
dedicated file? Wow!

In any case, please feel free to commit anything that fixes mingw-w64
directly to nvm.

> I'm getting lots of 'warning: unused parameter' messages from g++:
>
> ../monotone/src/simplestring_xform.hh:51:61: warning: unused parameter 'thing' [-Wunused-parameter]
>  origin::type get_made_from<std::string>(std::string const & thing)

Yeah, lots of those. However, I personally postponed any such cleanup
work for after 1.1, as it's hardly increasing stability but may possibly
break things.

Or put another way: Getting rid of warnings hasn't been a release goal
for me.

Regards

Markus Wanner



_______________________________________________
Monotone-devel mailing list
[hidden email]
https://lists.nongnu.org/mailman/listinfo/monotone-devel

[Markus Wanner-2]

On 04/30/2014 11:19 AM, Václav Zeman wrote:

> On 30 April 2014 11:05, Stephen Leake wrote:
>> Stephen Leake <[hidden email]> writes:
>>> Stephen Leake <[hidden email]> writes:
>>>> Is there a way to mark 'thing' as unused? or do we have to disable that
>>>> warning with -Wno-unused-parameter?
>>>
>>> I found three answers on stack overflow:
>>>
>>> 1) comment out or delete the parameter name:
>>>
>>>     a) origin::type get_made_from<std::string>(std::string const & /* thing */)
>>>
>>>     b) origin::type get_made_from<std::string>(std::string const & )
>>>
>>> 2) cast it to void:
>>>
>>>     (void)thing;
>>>
>>> I prefer 1a; it most clearly documents that we know there is a parameter
>>> by that name, but we are not using it in this case. Any objections?

I agree.

>> In most uses of CMD_NO_WORKSPACE, "execid" is not used, but it is used
>> in some (cmd_netsync.cc clone).

Yeah, macros are problematic - not only in that regard. I'd generally
like to see us move away from those, but...

> You could special case GCC and introduce your own macro that expands
> to the "unused" attribute in case of GCC:
> http://gcc.gnu.org/onlinedocs/gcc-4.9.0/gcc/Function-Attributes.html#index-g_t_0040code_007bunused_007d-attribute_002e-3000

Special casing a compiler just to get rid of a warning sounds weird to
me. How about other compilers? And changes between compiler versions?
We'll end up cluttering our code with a lot of ifdefs if we go that
route. That's not worth it, IMO.

That being said, I certainly agree we should try to reduce compiler
warnings as much as possible with generic ways. I started the branch
nvm.cleanup-warnings for that work. But please keep focusing on fixing
real issues for release 1.1, for now.

Regards

Markus Wanner



_______________________________________________
Monotone-devel mailing list
[hidden email]
https://lists.nongnu.org/mailman/listinfo/monotone-devel

[Stephen Leake-3]

Markus Wanner <[hidden email]> writes:

> On 04/30/2014 10:40 AM, Stephen Leake wrote:
>> I've succeeded in setting up a MinGW64 environment. It's quite nice once
>> you get past the tools install issues. I have not committed
>> INSTALL_windows_mingw64.txt yet; soon.
>
> Is is really that much different from mingw32 that it warrants a
> dedicated file? Wow!

Well, I started a separate file just in case. It turns out it is very
different; much simpler, since most of the dependent packages are
already packaged for MinGW64, and we can use 'pacman' to install them.

> In any case, please feel free to commit anything that fixes mingw-w64
> directly to nvm.

The 64 bit changes are mostly due to Windows using a different
underlying type for socket_type than Unix. There are many changes;
anything that checks for an invalid socket, some function return or
parameter types. The code is definitely cleaner after the fixes, so I
don't think it will break anything on 32 bit.
 
>> I'm getting lots of 'warning: unused parameter' messages from g++:
>>
>> ../monotone/src/simplestring_xform.hh:51:61: warning: unused
>> parameter 'thing' [-Wunused-parameter]
>>  origin::type get_made_from<std::string>(std::string const & thing)
>
> Yeah, lots of those. However, I personally postponed any such cleanup
> work for after 1.1, as it's hardly increasing stability but may possibly
> break things.

I've already done most of the work, but not commited or run the tests
yet; I'll commit it on a branch.

> Or put another way: Getting rid of warnings hasn't been a release goal
> for me.

I work much better when there are no spurious warnings; it's easier to
see the real problems in the compiler output.

So I'll add -Wno-unused-parameter to suppress the warnings, and take it
out on the branch.

--
-- Stephe

_______________________________________________
Monotone-devel mailing list
[hidden email]
https://lists.nongnu.org/mailman/listinfo/monotone-devel

[Markus Wanner-2]

Stephen,

On 05/01/2014 07:36 PM, Stephen Leake wrote:
> Well, I started a separate file just in case. It turns out it is very
> different; much simpler, since most of the dependent packages are
> already packaged for MinGW64, and we can use 'pacman' to install them.

pacman? Didn't read about that. Sounds good. The MinGW-w64 variant I
fetched had *less* than the 32-bit one. Which one do you use?

> The 64 bit changes are mostly due to Windows using a different
> underlying type for socket_type than Unix. There are many changes;

That worries me a tad.

> anything that checks for an invalid socket, some function return or
> parameter types. The code is definitely cleaner after the fixes, so I
> don't think it will break anything on 32 bit.

Phew, okay. Please try to keep the changes minimal.

> I've already done most of the work, but not commited or run the tests
> yet; I'll commit it on a branch.

Can you get this done by Friday night? I planned to release this weekend.

> I work much better when there are no spurious warnings; it's easier to
> see the real problems in the compiler output.
>
> So I'll add -Wno-unused-parameter to suppress the warnings, and take it
> out on the branch.

You can always do `./configure CXXFLAGS="-Wno-unused-parameter"` - then
there's nothing to take out.

Regards

Markus


_______________________________________________
Monotone-devel mailing list
[hidden email]
https://lists.nongnu.org/mailman/listinfo/monotone-devel

[Markus Wanner-2]

On 05/01/2014 09:20 PM, Markus Wanner wrote:
> You can always do `./configure CXXFLAGS="-Wno-unused-parameter"` - then
> there's nothing to take out.

Sorry, no, that doesn't work. Sigh!

(Due to -Wunused being appended after any flags passed in, thus
overriding that specific flag.)

Regards

Markus


_______________________________________________
Monotone-devel mailing list
[hidden email]
https://lists.nongnu.org/mailman/listinfo/monotone-devel

[Stephen Leake-3]

Markus Wanner <[hidden email]> writes:

> Stephen,
>
> On 05/01/2014 07:36 PM, Stephen Leake wrote:
>> Well, I started a separate file just in case. It turns out it is very
>> different; much simpler, since most of the dependent packages are
>> already packaged for MinGW64, and we can use 'pacman' to install them.
>
> pacman? Didn't read about that. Sounds good. The MinGW-w64 variant I
> fetched had *less* than the 32-bit one. Which one do you use?

I'm using the mingw64 packaged in msys2; msys2 includes 'pacman', which
is the package manager on one of the Linux distributions.

It is confusing; I would not have gotten it working without help from
the msys2 mailing list.

>> The 64 bit changes are mostly due to Windows using a different
>> underlying type for socket_type than Unix. There are many changes;
>
> That worries me a tad.

Yes, it worries me as well.

>> anything that checks for an invalid socket, some function return or
>> parameter types. The code is definitely cleaner after the fixes, so I
>> don't think it will break anything on 32 bit.
>
> Phew, okay. Please try to keep the changes minimal.
>
>> I've already done most of the work, but not commited or run the tests
>> yet; I'll commit it on a branch.
>
> Can you get this done by Friday night?

no.

>I planned to release this weekend.

Ok; go ahead. I'll create an nvm.msys2-mingw64 branch.

--
-- Stephe

_______________________________________________
Monotone-devel mailing list
[hidden email]
https://lists.nongnu.org/mailman/listinfo/monotone-devel

[Stephen Leake-3]

Markus Wanner <[hidden email]> writes:

> On 05/01/2014 09:20 PM, Markus Wanner wrote:
>> You can always do `./configure CXXFLAGS="-Wno-unused-parameter"` - then
>> there's nothing to take out.
>
> Sorry, no, that doesn't work. Sigh!
>
> (Due to -Wunused being appended after any flags passed in, thus
> overriding that specific flag.)

Yes, configure can be a pain.

I'll just branch nvm.msys2-mingw64 from nvm.cleanup-warnings, and bypass
this issue :).

--
-- Stephe

_______________________________________________
Monotone-devel mailing list
[hidden email]
https://lists.nongnu.org/mailman/listinfo/monotone-devel

[Stephen Leake-3]

Markus Wanner <[hidden email]> writes:

> That being said, I certainly agree we should try to reduce compiler
> warnings as much as possible with generic ways. I started the branch
> nvm.cleanup-warnings for that work. But please keep focusing on fixing
> real issues for release 1.1, for now.

I've built nvm.cleanup-warnings with 32 bit Mys2/mingw64 (confusing
names!). I fixed all the warnings (except one that needs
nvm.msys2-mingw-64).

All tests pass on msys2, except some extra tests. I can run it on Debian
later this weekend.

synced.

--
-- Stephe

_______________________________________________
Monotone-devel mailing list
[hidden email]
https://lists.nongnu.org/mailman/listinfo/monotone-devel