Warnings for unused variables
It's actually not a bad idea. Mind pushing the macros here? I think I could use them for my own projects. :)
Re: Warnings for unused variables
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 sure if the old code relied on undefined behavior or if Clang simply miscompiled it. The macro - function transformation did not really alter the semantics other than introducing volatile attributes so if the new form breaks gcc I suppose we were just lucky with the old one. That said, I think it makes sense to use something which does _not_ involve any undefined behavior. Agreed. If people mind the 7.9 instability with gcc I can revert the bignum patch since Clang isn't default even on OS X 10.8, but maybe we can give it a few days first.
Warnings for unused variables
Didn't intruduce any new warnings in Coverity.
Warnings for unused variables
Can't we just remove most of the unused arguments?
Warnings for unused variables
What is DEBUGUSED?
Warnings for unused variables
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.
Warnings for unused variables
(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?)
Warnings for unused variables
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.
Warnings for unused variables
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 (precompile.pike) which probably is a candidate for 7.8 backport as well but I'll let Grubba weigh in on that.
Re: Warnings for unused variables
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 overflow detection failures now using gcc. Its a little bit unfortunate that there is no default way to handle this. I would propose to use 128bit mult when available and fall back to manual multiplication otherwise. Unless someone has an easy way to fix this. Any ideas? arne On Sun, 30 Dec 2012, Jonas Walld?n @ Pike developers forum wrote: 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 (precompile.pike) which probably is a candidate for 7.8 backport as well but I'll let Grubba weigh in on that.
Re: Warnings for unused variables
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 every Pike variable adjustment (machine-code excepted) so if we can optimize that it would be great. Here's a link that I found that could be useful: https://www.securecoding.cert.org/confluence/display/seccode/INT32-C.+Ensure+that+operations+on+signed+integers+do+not+result+in+overflow?showComments=false
Re: Warnings for unused variables
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 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 every Pike variable adjustment (machine-code excepted) so if we can optimize that it would be great. Here's a link that I found that could be useful: https://www.securecoding.cert.org/confluence/display/seccode/INT32-C.+Ensure+that+operations+on+signed+integers+do+not+result+in+overflow?showComments=false 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. That said, I think it makes sense to use something which does _not_ involve any undefined behavior. Thanks for the link. I will have a look at that. I was thinking about using __int128 for those compilers that support it and manual multiplication otherwise (see http://www.fefe.de/intof.html). arne
Warnings for unused variables
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 in these examples: - static void exit_memory(struct object *o) + static void exit_memory(struct object *UNUSED(o)) and - static int got_udp_event (struct fd_callback_box *box, int event) + static int got_udp_event (struct fd_callback_box *box, int DEBUGUSED(event))
Unused variables
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.
Re: Unused variables
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 has?
E.g. int somefunc(int __attribute__(unused) level, int depth) { }
in order to suppress warnings for int level.
--
Sincerely, [EMAIL
PROTECTED]
Stephen R. van den Berg.
A sign seen at the local pizza place: DO NOT CARRY TAKE-OUT BOXES BY HANDLES
Re: Unused variables
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.
Unused variables
Not for strict types, no. Unless it was done yesterday.
Re: Unused variables
When is there a point of warning of unused arguments to a function? Have an __attribute__(warn_if_unused) instead? ;)
Re: Unused variables
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 fiddling with things. Plus deprecated arguments in APIs, but the compiler already have the deprecate flag to hang that on.
Re: Unused variables
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.
Re: Unused variables
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.
Re: Unused variables
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.
Re: Unused variables
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 fine, but if you have moved it to production and
start turning on #strict_whatever it time to lean more towards ADA
than Perl.
Re: Unused variables
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.
Re: Unused variables
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 leaving out the name is a good way, though) it's better to just leave it unimplemented.
Re: Unused variables
Since you where the champion of the idea we'll happily resolve this without action then.
Unused variables
I predict lots of warnings for mutex key objects in the future...
Unused variables
Good point. It would have to be restricted to either noncomplex types or side-effect free initializations, I guess.
Unused variables
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 disabled
for pure declarations).
In such cases I'd be fairly satisfied with the option of providing a
more complete function declaration using @decl in a refdoc comment.
If worst comes to worst, it's always possible to give a name in a
comment instead:
string chop_chop (object /*context*/, string data) {return data;}
Unused variables
We can have this discussion for 7.10. I'm tentatively against it.
Unused variables
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.
Unused variables
I really like getting warnings for non-argument unused variables.
