I'm in favor of the change. However, it's been pointed out to me in the past that with all object declarations at the start of the function body it's easier to reason about the stack.

In D25312#558219, @arichardson wrote:

In D25312#558211, @pstef wrote:

I'm in favor of the change. However, it's been pointed out to me in the past that with all object declarations at the start of the function body it's easier to reason about the stack.

Except that it isn't true unless you compile at -O0: any vaguely recent compiler will reuse stack slots for variables for which it knows the lifetime cannot overlap.

Thanks for this change!

I was probably vague. I referred to code and not to its execution. Consider this:

int f(void) {
  char *x;
  x = malloc(...);
  ...
  ...
  ...
  if (condition) {
    char buffer[huge];
  }
  ...

I wrote if (condition) { but that's more debatable than for (;;) { while both camouflage stack usage.

I do not like it.

I agree that the locals in the inner block indeed allow easier _write_ of the code, but I found empirically that it is easier to _read_ when locals are collected at the start of the function. Perhaps it is because I can see all local parameters in one place.

The argument that we have around a hundred places where the rule is already broken in fact confirms that most of our code confirms to the style(9) in this regard, since hundred places for several MLoCs of our own code is quite good ratio.

And, taking one step further, I strongly disagree about lifting the requirement of no initializators in local declarations.

I find this change debatable, and as presented here incomplete as I believe this has effects on some other parts of style(9), nor does it provide a proper sample (it leaves the example unchanged) Lets not RUSH to slam this in the tree!

In D25312#558270, @kib wrote:

I do not like it.
...

Kib raises some valid points, and if in fact the actual usage in non-vendor code it may be best to proceed slowly and carefully.

In D25312#558270, @kib wrote:

I found empirically that it is easier to _read_ when locals are collected at the start of the function. Perhaps it is because I can see all local parameters in one place.

I agree with this. One look at the beginning of a function definition and you have a good idea what the stack will look like. As opposed to when variables are declared somewhere later in the function.

I like this change; we've had nested variable declarations since 1989 and I think 30 years is more than enough time to get used to them. I'm a bit more cautious about declarations in the initializer of a for loop. I do acknowledge the concerns several developers have expressed above about stack depth; this information should be possible to derive from DWARF2 debugging information, so perhaps an automated tool could replace manual inspection of the source.

BTW, one thing this leaves ambiguous is our you can declare variables in the middle of blocks vs at the beginning. That is:

int
foo(int *p, size_t len)
{
    for (size_t i = 0; i < len; i++) {
        int one, two;

        one = p[i] * 2;
        two = bar(&one);
        ...
    }
}

vs.

int
foo(int *p, size_t len)
{
    for (size_t i = 0; i < len; i++) {
        int one = p[i] * 2;
        int two = bar(&one);
        ...
    }
}

I guess this is the second point kib@ made. Do we think relaxing this means it's ok to declare variables at the start of any block or do we think it's fine to define them in the middle of blocks? If the former you would perhaps want to replace the removed sentence with something like:

Place declarations at the start of blocks.

I had read the intention of removing this as being the former and not the latter FWIW.

In D25312#558449, @jhb wrote:

BTW, one thing this leaves ambiguous is [whether] you can declare variables in the middle of blocks vs at the beginning. That is:

[...]

Place declarations at the start of blocks.

I could live with this. Although I don’t have an objection to _allowing_ declarations in line when it makes sense, I do think one can get carried away and overdo it.

I'm ok dropping the sentence as-is, but I am concerned the increased use of scoped declarations could decrease legibility, if overused. Can we "lightly discourage" use of scoped declarations in a more relaxed way? The existing language seems to kind of do that if you read the qualification at the end and don't stop at "Do not …"

Maybe "Declarations in block scope are acceptable if they increase code clarity. Absent other factors, top-level declarations are preferred."? That is essentially the same as "use it when you want to," with a slight bias towards top-level declarations. I'm ok being outvoted on this one.

In D25312#558449, @jhb wrote:

BTW, one thing this leaves ambiguous is our you can declare variables in the middle of blocks vs at the beginning. That is:

int
foo(int *p, size_t len)
{
    for (size_t i = 0; i < len; i++) {
        int one, two;

        one = p[i] * 2;
        two = bar(&one);
        ...
    }
}

vs.

int
foo(int *p, size_t len)
{
    for (size_t i = 0; i < len; i++) {
        int one = p[i] * 2;
        int two = bar(&one);
        ...
    }
}

I guess this is the second point kib@ made. Do we think relaxing this means it's ok to declare variables at the start of any block or do we think it's fine to define them in the middle of blocks? If the former you would perhaps want to replace the removed sentence with something like:

Place declarations at the start of blocks.

I had read the intention of removing this as being the former and not the latter FWIW.

comma statements should be avoided:)

In D25312#558466, @dab wrote:

In D25312#558449, @jhb wrote:

BTW, one thing this leaves ambiguous is [whether] you can declare variables in the middle of blocks vs at the beginning. That is:

[...]

Place declarations at the start of blocks.

I could live with this. Although I don’t have an objection to _allowing_ declarations in line when it makes sense, I do think one can get carried away and overdo it.

+1

I wonder if we need a poll for this, since there are now three options (1: as-is, 2: start-of-block, 3: anywhere). The ambiguity means most of the approvals are also ambiguous.

That being said, since the majority is clearly in favor of some change, you could simply commit this as-is, and someone could open a followup review to distinguish options 2 and 3. There is no need to be overly legalistic.

In D25312#558372, @pstef wrote:

One look at the beginning of a function definition and you have a good idea what the stack will look like. As opposed to when variables are declared somewhere later in the function.

I'm afraid that's not the case. Some of this was already mentioned above, but I'll summarize in one place.

Many modern machines have a lot of general purpose registers, which the compiler will use for some local variables. Looking at a declaration block alone doesn't tell you which will be where (except those too large for a register). You have to study the entire function, and even then, you might be surprised.

Callees might be inlined into the function of interest, merging the stack frames, further complicating a human's understanding of the size and layout.

Modern compilers track variable liveness, so one storage location might hold multiple variables at different times. This argument is in favor of this change, since minimizing the scope of a variable more closely represents this liveness tracking.

In D25312#558827, @vangyzen wrote:

In D25312#558372, @pstef wrote:

One look at the beginning of a function definition and you have a good idea what the stack will look like. As opposed to when variables are declared somewhere later in the function.

I'm afraid that's not the case. Some of this was already mentioned above, but I'll summarize in one place.

Many modern machines have a lot of general purpose registers, which the compiler will use for some local variables. Looking at a declaration block alone doesn't tell you which will be where (except those too large for a register). You have to study the entire function, and even then, you might be surprised.

Callees might be inlined into the function of interest, merging the stack frames, further complicating a human's understanding of the size and layout.

Modern compilers track variable liveness, so one storage location might hold multiple variables at different times. This argument is in favor of this change, since minimizing the scope of a variable more closely represents this liveness tracking.

Note that what I said has nothing to do with the stack usage. I mean exactly the cognitive load to assess the local data that is operated by the function upon.
I am very much aware about stack space reuse optimizations due to reading a lot of disassemble looking for panics reasons.

Summary so far:

1. there's overwhelming support for this, at least in theory
2. the devil is in the details.

As constructed, it's a bit incomplete, however. There's a couple of other tweaks likely necessary to go along with this. So I'll be doing a second round that's more verbose and offers some best practices clarifications.

The millions of lines of code is an unfair denominator: only a tiny fraction of those are declarations. And vendor code should not be excluded since it represents industry practice. I know I've written maybe thousands of extra lines of code with newbus because I couldn't say struct foo_softc *sc = device_get_softc(dev); or because I passed in a void *xfoo and wanted to use foo_t *foo = xfoo; There's much pent up demand for this. There's wide-spread support for this, I judge, though I'll make sure the details have a chance for approval of people that are commenting here.

Stack usage is a red herring. No modern compiler cares and above -O0 you can't predict anything.

Random declarations mixed in with code are rare in the tree, and rarely promote better code. The support for these isn't clear and likely should be handled separately.

So the goal of any style doc is not only prescriptive, but to encode best practices. With that in mind, I propose to come up with a new diff that codifies the following:

1. Initializations may be included in declaration where it makes the code clearer
2. Weaken the admonition against nested variables
3. Explicitly allow for (int i...) but I'm on the fence for the examples.

All these things exist today in the tree and in many places make the code more compact, readable and understandable (though I'm sure you'll find places too that do not). I was naive to think the originally proposed simple change would suffice.

In D25312#559159, @imp wrote:

Summary so far:

1. there's overwhelming support for this, at least in theory
2. the devil is in the details.

... Much sound analysis removed ...

Thank you for this summary, I agree that it represents the consensus and concerns raised by all and that doing so would achieve a better result.

In D25312#564662, @jhb wrote:

I think the rule to only use initializers when the value is constant is probably a good compromise.

Yea. I phrased it like that because that seemed to be what the vast majority of the tree does and therefore had implicit support of the community of developers... I'm open to other ways to capture this, but I think this is the best compromise (though we didn't talk about it in the review itself).

Though much better than the prior iteration I believe it leaves some detail out.