It's actually not a bad idea. Mind pushing the macros here? I think I
could use them for my own projects. :)
As far as I can see its the latest change, which turned the macro
into fuctions. I think its overall quite mysterious under which
circumstances gcc (or clang) optimize that check away. In particular
because compilers change and something that works now might break
tomorrow.
I don't know for
Didn't intruduce any new warnings in Coverity.
Can't we just remove most of the unused arguments?
What is DEBUGUSED?
I assume it's something that expands to UNUSED only if not making a
debug build, since the use of the variable is inside #ifdef DEBUG.
(For gcc this is a bit overkill since you can still use a varabile
declared as __attribute__((unused)) without getting a warning, but
maybe it is needed for clang?)
The macro I'm borrowing renames variables to prevent accidental use.
There are still places where variable usage is conditioned on other
defines (e.g. POLL_DEBUG) and I'll leave them unchanged instead of
creating a more complex wrapper.
Pushed now. If anything breaks please let me know.
This cleanup was kickstarted when I tried to compile 7.9 with Clang. I
still see wrong execution in e.g. integer/bignum handling so I hope to
have more patches in the coming days. One of my other findings was the
commit aabfb4f04c5
One issue I run into is that the definition of UNUSED clashes with a
similar definition used in libvpx which is used in the webp module.
Not sure if that is enough reason to rename the macro, instead of
working around it locally..
Even though its unrelated, but since you are asking: I am getting
Too bad with the name clash, but a Pike-prefixed name would also be
quite annoying due to its length.
Do you mean that my latest changes introduced the gcc overflow issues
or only that you noticed them as of now? I'm also concerned about the
overflow detection method using costly integer division
On Sun, 30 Dec 2012, Jonas Walld?n @ Pike developers forum wrote:
Too bad with the name clash, but a Pike-prefixed name would also be
quite annoying due to its length.
Yes I agree. I will add an undef before the libvpx includes, which
should fix that.
Do you mean that my latest changes
When compiling Pike with clang I get massive amounts of warnings for
unused variables. This detracts quite a bit from finding more serious
errors and I've got a patch ready to remove them. However, before
pushing that I'd like to hear if people have objections to changing
declarations like
An all-on/all-off switch doesn't sound particularly useful. It's for
each individual argument one wants to make the decision whether it's
intended to remain unused or not.
Martin Stjernholm, Roxen IS @ Pike developers forum wrote:
An all-on/all-off switch doesn't sound particularly useful. It's for
each individual argument one wants to make the decision whether it's
intended to remain unused or not.
What about something similar to __attribute__(unused) like gcc
Only problem is that something like that would be exceedingly clumsy.
It would force everyone to invent his own little macro (since pike by
convention doesn't have a common include file system), which would
obscure the meaning.
Not for strict types, no. Unless it was done yesterday.
When is there a point of warning of unused arguments to a function?
Have an
__attribute__(warn_if_unused)
instead? ;)
For most functions the warning makes sense in strict mode. That's when
you should have a clean, finished function with the cruft removed.
The exceptions become painful though: callbacks and #ifdefs. And
sometimes when you develop in strict mode, because you will have
unused arguments while
No you shouldn't because that would suck. that's why it should be
turned on with a separate #pragma for now. I'm just saying that if
possible to make non-irritating the warning is nice in most cases.
I'm not sure if I see arguments, even unused, as cruft. They are
usually both for method compatibility and/or documentation when they
aren't used, and that's not usually something that you want to remove
just because it's a finished product.
Take it from the other viewpoint: What kind of errors do you detect
with that warning? I don't see many that is worth that warning.
Warnings about unused function-local variables, though. That catches
many errors and they don't fill any function unused.
void remove(string file, int(0..1) dry_run)
{
werror(Now removing %O\n, file);
// if(!dry_run)
rm(file);
}
Might be somewhat contrived, but leaving unused arguments like this
primarily screws up readability when scanning code. When you develop
it it's perfectly
But as I mentioned eariler, it's currently impossible in pike, and
it's also impossible in general, to write callback based API:s without
having unused arguments. Unless you only want a single mapping or
event object as the argument.
It's not acceptible with a #pragma either. That's way too clumsy.
I don't think a warning for unused arguments is that useful to begin
with; the risk to accidentally miss using an argument is not very
high. So if we can't agree on a good way to turn it off on a
per-argument basis (I still think
Since you where the champion of the idea we'll happily resolve this
without action then.
I predict lots of warnings for mutex key objects in the future...
Good point. It would have to be restricted to either noncomplex types
or side-effect free initializations, I guess.
You mean people would feel forced to leave out the argument name just
to silence the compiler?
I can only see that happening for functions in interface classes that
defines some kind of callback, and that for some reason need to have a
(more or less) dummy body (the warning should of course be
We can have this discussion for 7.10. I'm tentatively against it.
I think we've had this discussion a while ago an decided agains
nameless parameters.
I'd rather have unused arguments become an optional warning in the
compiler than any other wierd workaround. That way, I can detect them
if I want to but for the most part just ignore them.
I really like getting warnings for non-argument unused variables.
32 matches
Mail list logo
