www.digitalmars.com         C & C++   DMDScript  

digitalmars.D - Re: Wish: Variable Not Used Warning

reply Era Scarecrow <rtcvb32 yahoo.com> writes:
 Manfred_Nowak wrote:
 JAnderson wrote:
 
 The more warnings as errors the better.  If I have


 little for false positives *shrug*

What do you understand by "a little"?

I don't understand what your asking. I meant that if I have to fix it because the compiler tells me its an error then so be it. Its a little pain for a lot of gain.

Forcing all warnings to be errors will not always be nice or pretty in any given circumstance. And occasionally, the 'error's will appear on certain architectures and not others, am i right? Under a basic understanding, the compiler does the following at the start of a function/code block. First prepares the stack with space, then initializes if it's static, Etc. With an unused variable, it shouldn't be an error during certain phases, and should be on others. Example. During my first few phases i usually start by declaring my function, start by what it needs, the order of the parameters and names, then i put in several different variables declaring them. //basic declaration to be finished later int isPrime(int number, int[] primesList) { int cnt; return 0; //shut the compiler up for now. } simply as an example, i can declare primesList, which i intend to implement at a later time for performance and speed, but at the moment i'll implement a very basic formula that is simple and requires very little work. What's the worst case that primesList or cnt aren't used right now? 8? 12 bytes in the stack? Geez, you all make it sound like it should be an error and it's illegal to not touch something now because i'm working on it and other things at once. I think i'd rather have a simple warning saying 'oh by the way, you aren't using this' when i go through my warnings/error list and i'll say later on down the road 'oh yeah, here's something i need to finish' or 'oh i don't need that variable, let's remove it' rather than.... //basic declaration to be finished later int isPrime(int number, int[] primesList) { int cnt; unusedReferenceToShutUpErrors(cnt); unusedReferenceToShutUpErrors(primesList); return 0; //shut the compiler up for now. } In my opinion that extra code is just taking up space to tell the compiler to shut the hell up, with no functional usage what-so-ever. Please don't say it isn't, i'm not going to fight and reply on the issue. I don't see why everyone is so nit-picky about this specific topic. My opinion? If the variable isn't used, have it as a warning but compile it, if you set warnings as errors, you can remove/comment the declarations when the time comes but i always work to get the code working before i re-factor it to make the 'by the ways' warnings go away, if they need it. I'm not sure about everyone else, but i've been reading a lot on how Walter has been defining and building D. The way it works, usage, declarations; There's a few minor things i am not sure yet about (bitfields, multi-dimensional arrays, Ect). However it's a big stepup from C, and looks much easier to understand without ugly operators and overloading that C++ has (::, friend functions, headers and preprocessing, Ect.). It was these very things that kept me from learning C++, because it was too hard for me to grasp because it was ugly, and i don't want to make something ugly.
 Please look at the example from 
 http://www.digitalmars.com/webnews/newsgroups.php?
 art_group=digitalmars.D&article_id=73441 
 
 Do you recognize how many warnings a lint tool might

 Would you admit then, that a paranoic lint would be

 if it detects that the variable `p' should be

I don't understand? With lint it just gives you hints about what could be wrong. You pick and choose what to fix.
 Would you 
 admit, that you yourself are unable to decide whether

 some access statements to `p' should suppress the

I would prefer this be an error like C#. In C++ because all my warnings are errors it would be an error too. If you really want to use an uninitialized variable there should be a work around but it should be harder to do.

Indeed. But some warnings you can't ignore without dropping out the code. Take the example. const DEBUG = 0; .. if (DEBUG) { //put debugging information } The compiler SHOULD give you a happy warning of 'Always comes up False' or 'Always comes up True' when you're enabling/disabling code in this manner. Making all warnings as errors, simply removes the functionality of the code. I believe GCC does a good job at it, last i checked, and occasionally notice i simply did the equation wrong when i see the warning where it shouldn't be there. while(1) //always true { if (someCondition){ break; } } I've had to use this code once or twice, once again you'll have a warning, not strictly an error. Era
Jul 10 2008
next sibling parent reply Markus Koskimies <markus reaaliaika.net> writes:
On Thu, 10 Jul 2008 04:16:06 -0700, Era Scarecrow wrote:

[...]
 //basic declaration to be finished later int isPrime(int number, int[]
 primesList) {
     int cnt;
 
     return 0; //shut the compiler up for now.
 }

 //basic declaration to be finished later int isPrime(int number, int[]
 primesList) {
     int cnt;
     unusedReferenceToShutUpErrors(cnt);
     unusedReferenceToShutUpErrors(primesList);
 
     return 0; //shut the compiler up for now.
 }

In "C++'ish" / D way, this is normally dealt like this: int isPrime(int number, int[] /*primesList*/) { //int cnt; return 0; } Hard & ugly? I think warnings are not meant to be used to remember you that you have something unfinished. I regularly tag those parts with "// todo", which is easily grep'd from code.
  What's the worst case that primesList or cnt aren't used right now? 8?
  12 bytes in the stack? Geez, you all make it sound like it should be an
  error and it's illegal to not touch something now because i'm working
  on it and other things at once.

It is not about consuming memory, it's about compiling code that won't work. It is not about making intentionally dead code, it's about accidentally having dead code.
  I don't see why everyone is so nit-picky about this specific topic.

Because syntactic salts and more pedantic checkings save a lots of debugging time.
  It was these very things that kept me from learning C++, because it was
  too hard for me to grasp because it was ugly, and i don't want to make
  something ugly.

Warnings & checkings does not make C++ ugly :) Constant conditionals:
 const DEBUG = 0;
 
 ..
 
 if (DEBUG) {
     //put debugging information
 }

1) Use static if? 2) Anyway, it is not constant conditional in that way that it is normally warned - you have intentionally set the value so, and thus the compiler could optimize the dead code away without problem. Normally it is warned, if you have an expression: for(...; (a + b) < 8; ...) { ... } ...if the compiler recognizes, that the condition cannot never be anything else than true or false, do you think it is intentional?
 while(1)    //always true
 {
     if (someCondition){
         break;
     }
 }

The same hold here; "while(true)", "for(;;)" are intentionally written to be infinite loops. I regularly define in C/C++: #define forever for(;;) I really don't mean, that the "warning system" should be something like "code smelling analyzing" for refactoring, but I think that most of those basic things could easily be catched at compile time (w/ warnings) without any negative side effects.
Jul 10 2008
parent reply "Nick Sabalausky" <a a.a> writes:
"Markus Koskimies" <markus reaaliaika.net> wrote in message 
news:g54tke$1h9i$11 digitalmars.com...
 On Thu, 10 Jul 2008 04:16:06 -0700, Era Scarecrow wrote:

 [...]
 //basic declaration to be finished later int isPrime(int number, int[]
 primesList) {
     int cnt;

     return 0; //shut the compiler up for now.
 }

 //basic declaration to be finished later int isPrime(int number, int[]
 primesList) {
     int cnt;
     unusedReferenceToShutUpErrors(cnt);
     unusedReferenceToShutUpErrors(primesList);

     return 0; //shut the compiler up for now.
 }

In "C++'ish" / D way, this is normally dealt like this: int isPrime(int number, int[] /*primesList*/) { //int cnt; return 0; } Hard & ugly? I think warnings are not meant to be used to remember you that you have something unfinished. I regularly tag those parts with "// todo", which is easily grep'd from code.

Warnings are intended to point out things you may have overlooked. Forgetting to finish something certainly qualifies as "overlooked". I do "//TODO"s as well, but I do it so much that I'm very likely to end up forgetting to finish a partial-implementation before I get around to going through my TODOs and getting to that particular one. So the problem that inevitably crops up is: When I've commented that stuff out to make my partial implementation compile (so that I can test what I've implemented so far), and then move on to something else and forget to come back to my partial implementation, what's going to happen? It's NOT going to give me a nice noisy warning about "Hey, Mr. Screwup, you're not using this variable!", which *would* have pointed me right back to my unfinished code *before* I got around to that particular "//TODO". So now I have a hidden bug on my hands, just because I've deliberately circumvented the warning system and thereby lost the benefits of having it.
Jul 10 2008
parent "Nick Sabalausky" <a a.a> writes:
"Nick Sabalausky" <a a.a> wrote in message 
news:g55ovu$1p0b$1 digitalmars.com...
 "Markus Koskimies" <markus reaaliaika.net> wrote in message 
 news:g54tke$1h9i$11 digitalmars.com...
 On Thu, 10 Jul 2008 04:16:06 -0700, Era Scarecrow wrote:

 [...]
 //basic declaration to be finished later int isPrime(int number, int[]
 primesList) {
     int cnt;

     return 0; //shut the compiler up for now.
 }

 //basic declaration to be finished later int isPrime(int number, int[]
 primesList) {
     int cnt;
     unusedReferenceToShutUpErrors(cnt);
     unusedReferenceToShutUpErrors(primesList);

     return 0; //shut the compiler up for now.
 }

In "C++'ish" / D way, this is normally dealt like this: int isPrime(int number, int[] /*primesList*/) { //int cnt; return 0; } Hard & ugly? I think warnings are not meant to be used to remember you that you have something unfinished. I regularly tag those parts with "// todo", which is easily grep'd from code.

Warnings are intended to point out things you may have overlooked. Forgetting to finish something certainly qualifies as "overlooked". I do "//TODO"s as well, but I do it so much that I'm very likely to end up forgetting to finish a partial-implementation before I get around to going through my TODOs and getting to that particular one. So the problem that inevitably crops up is: When I've commented that stuff out to make my partial implementation compile (so that I can test what I've implemented so far), and then move on to something else and forget to come back to my partial implementation, what's going to happen? It's NOT going to give me a nice noisy warning about "Hey, Mr. Screwup, you're not using this variable!", which *would* have pointed me right back to my unfinished code *before* I got around to that particular "//TODO". So now I have a hidden bug on my hands, just because I've deliberately circumvented the warning system and thereby lost the benefits of having it.

In other words, treating warnings as errors (at least if it's all the time) or turning warnings into errors creates a need to supress the warnings. And in cases like these, supressing them ends up defeating the whole point of having them in the first place.
Jul 10 2008
prev sibling next sibling parent Markus Koskimies <markus reaaliaika.net> writes:
On Thu, 10 Jul 2008 12:00:15 +0000, Markus Koskimies wrote:

An addition; I wouldn't mind (in fact I would hail) if all unused parts 
would be declared as an error in D spec. Unused/uninitalized vars, 
private members, dead code blocks and such are normally produced when you 
have had an intensive coding session to change some internals, but you 
have not remembered to change the behavior in every affected places. At 
least I never intentionally do dead code, they always mean that I have 
forgotten to do something. Furthermore, D has so many powerful tools for 
quickly "out-commenting" dead code (like "/+ ... +/" and static if) that 
it should not be a problem.

There are some harder-to-detect -things, just like infinite loops. I 
think that it is always an error to make a compiler-detectable infinite 
loop (or if statement) using comparison operators (even if there are 
reachable breaks or exception-throwable calls inside of it), like:

	for(; -1 > 0 ;) { ... }

"for(;;)" is valid for those, as well as "while(true)". "if(true)" would 
be better to be written "static if(true/false)".

Signed-unsigned -comparison is easily unintentionally made, since most 
programmers use int extensively and library functions return size_t and 
similar types, which can be unsigned. IMO it is a clear programming 
error; 
Jul 10 2008
prev sibling parent Markus Koskimies <markus reaaliaika.net> writes:
On Thu, 10 Jul 2008 15:51:52 -0400, Nick Sabalausky wrote:

 In other words, treating warnings as errors (at least if it's all the
 time) or turning warnings into errors creates a need to supress the
 warnings. And in cases like these, supressing them ends up defeating the
 whole point of having them in the first place.

I understand your point, and that's why my suggestion would be that the compiler has flag to relax (warnings as warnings) when sketching the code :) (sure it is just the same if compiler is relaxed or strict by default, but the whole point is if the warnings are needed or not). The current DMD -release -flag would be good point to turn warnings to errors, but unfortunately it removes runtime checks :(
Jul 10 2008