3

My assumption is that this is going to mess with checkers and stack analysis. I can't prove my assumption and I don't think C99 will complain. Probably neither c89 will because the definition is immediately after the opening of the curly brace: 

 if(true == condition){
       int i = 0; 
       /* do stuff with i */
 }else{ 
   foo():
 }

The two paths will lead to different stack usage. Declaring i outside the if/else statement will lead to a more defined stack usage (ok, I am branching to foo,so the stack will not be exactly the same in the two cases). But Misra advises to limit the scope of a variable closest to its usage.

Am I overthinking it or could there be a rationale in my assumption?

Catosh
  • 163
  • 1
  • 2
  • 14
  • 13
    This is considered good style and I doubt your claims about stack usage – M.M Dec 18 '18 at 09:18
  • Ok,I accept your point but I would like that you argument a bit about stack usage. I'm really interested in your perspective on this. – Catosh Dec 18 '18 at 09:38
  • 4
    It may be instructive to look at the assembly code generated by the compiler. What I expect you'll see is that the compiler sets up the stack frame once, reserving enough space for the worst case usage. – user3386109 Dec 18 '18 at 09:43
  • 1
    Do you have some evidence that the stack usage within the different branches is different in any piece of coude you looked at? I would not be surprised if the compiler just allocates the maximum of all branches already when you enter the function. It makes it easier e.g. to return from within any branch without having to deal with different stack usage. – Gerhardh Dec 18 '18 at 10:14
  • No, I haven't. It makes sense that the compiler allocates resources for the worst case scenario. – Catosh Dec 18 '18 at 10:25
  • `if(true == condition)` is bad style. First of all, you should never use the "Yoda conditions", that's a confused rule from the 1980s that's been obsolete for 30 years. Instead, listen to the compiler warning about assignment inside conditions. Second, booleans should be checked as `if(condition)`, comparing them against true/false only adds clutter. – Lundin Dec 18 '18 at 12:26
  • @Lundin avoiding the "Yoda conditions" it's because they look awful or for some other practical reason? I take care of all compiler warnings, but actually I am more reactive to compiler errors when developing. – Catosh Dec 18 '18 at 16:23
  • 2
    @Catosh Yoda conditions are fairly common, that guy is being overly prescriptive – M.M Dec 18 '18 at 23:05
  • 1
    @Catosh This is not the place for that debate (Google "why are Yoda conditions bad"), but basically compilers invented a warning "possible incorrect assignment" with the advent of Turbo C in 1989, rendering the Yoda conditions obsolete. Best practice is to never use assignment inside conditions, allowing compilers/tools to detect typos. The most important argument against Yoda is: "If you can remember to swap the order of the operands, you may as well remember to double-check for == vs =". – Lundin Dec 19 '18 at 07:50
  • @Lundin *The most important argument against Yoda is: "If you can remember to swap the order of the operands, you may as well remember to double-check for == vs =".* But then you need to remember to remember.... – Andrew Henle Dec 19 '18 at 11:13
  • @AndrewHenle Or you can install Turbo C from 1989 and have it remember it for you. I wouldn't advise to use a compiler with worse diagnostics than TC from 1989, such as for example `icc -Wall -Wextra` (icc 19). gcc and clang also have bad diagnostics, warning for `if(a = b)` but not `if((a = b) || (b == c))`. These are embarrassing shortcomings of the mainstream compilers. While various embedded systems compilers give the expected diagnostics, as do hobbyist compilers like VS. – Lundin Dec 19 '18 at 12:37
  • @Lundin Your argument that mainstream compilers provide bad diagnostics for assignments in conditions contradicts your earlier argument that Yoda conditions are obsolete. And I do agree with you that performing assignments in conditions is a horrible, horrible coding style. It creates bugs and impairs proper error handling. The "brevity of code" argument in favor of it is just cargo-cult garbage. – Andrew Henle Dec 19 '18 at 12:55
  • @AndrewHenle There's no contradiction: you shouldn't use Yoda conditions nor should you use compilers that are worse than ancient compilers like TC or VS2017, both from the year 1989. gcc and clang give acceptable diagnostics - though I've always said that the double parenthesis disabling warnings is to be regarded as a compiler bug. – Lundin Dec 19 '18 at 13:00
  • Back on topic... there is nothing wrong with defining variables at the lowest scope - in fact, even if both the `if` and the `else` clauses use the variable, but they are distinct usages, it would make sense to declare them separately at each block. – Andrew Dec 22 '18 at 17:09
  • PS: Please do not blame MISRA for things that MISRA do not say ;-) – Andrew Dec 22 '18 at 17:09

2 Answers2

8

Is declaring a variable inside an if statement in c a bad habit?

No.

A modern approach is to minimize the scope of the variables used, so that logical (hard-to-fix) and syntactical (easy-to-fix) errors are avoided.

Of course, there are people that still like to see all the variables defined at the topmost part of the code, because this was the convention in the past, as @Clifford commented.

BTW, your code should compile fine, both with C89 and C99.

This stack usage thought is the result of overthinking, and I suggest you follow the Ancient Hellenic phrase: Métron áriston.

gsamaras
  • 71,951
  • 46
  • 188
  • 305
  • Even K&R C allows a declaration list at the start of any _compound statement_ (_K&R 1st ed. Appendix A §18.3_), so it was merely a matter of convention rather than _"how it was done in past versions of C"_. The convention probably came about because in the same edition in §1.2 K&R state "[...] _all_ variables must be declared before use, **usually at the beginning of the function** [...]" (my **bold**), and because that is the case in all examples in the book. Only the compiler implementers typically read section 18 in detail perhaps, not most practitioners. – Clifford Dec 19 '18 at 10:51
5

The code is fine in any version of C (except C90 does not support true).

The two paths will lead to different stack usage.

This is mostly a myth. Modern compilers stack a variable if they can determine that it is needed, regardless of where you place the declaration.

If the variable is allocated in a register, then it will only be allocated when the program takes the path where your example declares the variable. This is not because of where the declaration is placed, but because that path will be executed. So again, for the sake of performance, it doesn't matter where the variable is declared, as long as it is somewhere in local scope and not at file scope.

It is good practice to limit the scope of variables as much as possible. But this is to avoid unintentional bugs and namespace collisions.

But Misra advises to limit the scope of a variable closest to its usage.

No it doesn't, but some static analysers require you to do that, on top of the MISRA requirement. Both MISRA-C:2004 8.7 and MISRA-C:2012 8.9 only require that you place a variable at block scope, if it is only used by one function. That's it.

MISRA does however say:

Within a function, whether objects are defined at the outermost or innermost block is largely a matter of style

Lundin
  • 195,001
  • 40
  • 254
  • 396