style guide: VLAs, function comments, true/false conditions, ...

[justinmk]

I believe we allow VLAs now, so we should modify the style guide to that effect.

Also since we now generate headers, we need to update the style guide passage regarding placement of function docstrings.

[jdavis]

Cool. Your wish is my command, Justin. I'll update it soon =]

[justinmk]

Another thing we may want to add to the style guide: https://github.com/neovim/neovim/pull/1023#discussion_r15731634

That is, do we recommend this:

if (foo == false && bar == true) {
    ...
}

or this:

if (!foo && bar) {
    ...
}

Personally I prefer the latter.

[splinterofchaos]

Also, I've been considering asking whether of not we should

if (ptrA != NULL || ptrB == NULL) ...

vs

if (ptrA || !ptrB) ...

The former seems defacto-standard, but the style guide makes no mention of it.

[justinmk]

More things to note in the style guide:

• %zu format specifier #1729
• PRId64 instead of %ld, etc. #599

[fwalch]

• We autogenerate the header files, so the distinction between function declaration/definition comments does not apply https://github.com/neovim/neovim/pull/2284#discussion_r27564286

[justinmk]

• strcmp family of functions: prefer explicit comparison (strcmp(...) == 0 vs !strcmp(...) ?

[justinmk]

• prefer (foo) and (!foo) instead of (foo != NULL) and (foo == NULL), where foo is a pointer.
• prefer (foo == NUL) and (foo != NUL) where it is semantically appropriate.

[ghost]

prefer (foo) and (!foo) instead of (foo != NULL) and (foo == NULL), where foo is a pointer.

On one hand I think your suggestion is more idiomatic, but IMO it can make it harder to read code. If you encounter (foo == NULL) it's immediately obvious that it's a pointer, but (foo) is not. Of course (foo) could be in the presence of functions which imply it's a pointer, but this isn't necessarily obvious.

prefer (foo == NUL) and (foo != NUL) where it is semantically appropriate.

Is this not already done?

[justinmk]

On one hand I think your suggestion is more idiomatic, but IMO it can make it harder to read code.

I'm neutral, but it seems that our most prolific contributors are in favor of it.

Is this not already done?

You mean it's already in the style guide? I didn't check. We just need to update the style guide so people aren't confused.

[ZyX-I]

I think that == NULL/!= NULL checks are more readable as well. Also I cannot find where it is said that NULL must be zero:

7.17 Common definitions <stddef.h>

The macros are NULL which expands to an implementation-defined null pointer constant; and …

Note implementation-defined.

Later it is explicitly said that null pointer may be not the pointer whose value is all-zeroes:

7.20.3.1 The calloc function

The calloc function allocates space for an array of nmemb objects, each of whose size is size. The space is initialized to all bits zero.²⁶¹⁾

261. Note that this need not be the same as the representation of floating-point zero or a null pointer constant.

. On the other side

6.3.2.3 Pointers

An integer constant expression with the value 0, or such an expression cast to type void *, is called a null pointer constant. If a null pointer constant is converted to a pointer type, the resulting pointer, called a null pointer, is guaranteed to compare unequal to a pointer to any object or function.

I failed to find references which say that

1. Null pointer compares equal to zero. (The only condition on which converting to _Bool emits false, also condition on which various boolean expressions are assumed false.)
2. Null pointer converted to an integer type must emit zero (the above paragraph only says about the opposite: integer zero converted to a void * must be a null pointer).
3. Converting an integer to a pointer and back must emit the same result (it is only true for pointers in ptr -> u?intptr_t -> ptr, but not for integers in u?intptr_t -> ptr -> u?intptr_t: theoretically you may map one pointer to more then one different u?intptr_t integer value and use them randomly on conversion and this is going to work as long as no two pointers are converted to the same integer).
4. Converting a null pointer to intptr_t/uintptr_t must result in zero (even must be possible: the standard says that these types are optional).
5. An integer 0 which is not a constant expression or it cast to type void * must result in a null pointer constant.

So basically you must not use if (ptr) because it is not portable. If you know additional references which prove me wrong please show them.

[justinmk]

I think we're all aware of that, but any C compiler which defines NULL to anything other than 0 is guaranteed to not run any useful software. I don't feel strongly about it though.

[elmart]

@ZyX-I The standard may not guarantee that NULL representation is all zeros (though I don't know about any implementation that doesn't), but it is guaranteed that a null pointer (whatever its representation) will convert to false when evaluated in a boolean context. So, abbreviated form is perfectly safe.

[oni-link]

A faq for null pointers: http://c-faq.com/null/index.html. The problem with if (ptr) is answered in question 3.

[justinmk]

@oni-link From 5.3:

if(p) is equivalent to if(p != 0)

and this is a comparison context, so the compiler can tell that the (implicit) 0 is actually a null pointer constant, and use the correct null pointer value. ... The internal representation of a null pointer does not matter.

The C-faq is often 95% complete, and then stops just short of spelling out a crucial detail. Is it saying if NULL == 1, the compiler will resolve if(p) to if(p != 1)?

[oni-link]

The C-faq is often 95% complete, and then stops just short of spelling out a crucial detail. Is it saying if NULL == 1, the compiler will resolve the expression as if(p != 1)?

In the explanation NULL is not used. Also 1 would not be an integral constant expression with the value 0 (so cannot be a null pointer), see @ZyX-I reference:

6.3.2.3 Pointers An integer constant expression with the value 0, or such an expression cast to type void *, is called a null pointer constant. If a null pointer constant is converted to a pointer type, the resulting pointer, called a null pointer, is guaranteed to compare unequal to a pointer to any object or function

[elmart]

@justinmk No, it's saying the opposite. That the integer 0 will be converted to null pointer (whatever its representation) in order to be compared to p.

[ZyX-I]

On April 24, 2015 1:35:45 AM EAT, oni-link [email protected] wrote:

The C-faq is often 95% complete, and then stops just short of spelling out a crucial detail. Is it saying if NULL == 1, the compiler will resolve the expression as if(p != 1)?

In the explanation NULL is not used. Also 1 would not be an integral constant expression with the value 0 (so cannot be a null pointer), see @ZyX-I reference:

It can. This section says that ... is called a null pointer constant ... converted to a pointer type ... called a null pointer. It does not say that this is the only way to get a null pointer. I do not see anywhere definition which says that two equal pointers can only be obtained from equal integers should they be obtained from integers.

Note that on a 32-bit system ((void ) ((uint64_t) 0xFFFFFFFF00000000LL)) will be a null pointer so it would be very stupid from the standard authors to require anything like this. But I also do not see why 0x1 *and 0x0 converted to a pointer type cannot both be a null pointers. (I also do not know any systems where this actually happens though.)

6.3.2.3 Pointers An integer constant expression with the value 0, or such an expression cast to type void *, is called a null pointer constant. If a null pointer constant is converted to a pointer type, the resulting pointer, called a null pointer, is guaranteed to compare unequal to a pointer to any object or function

---

Reply to this email directly or view it on GitHub: https://github.com/neovim/neovim.github.io/issues/53#issuecomment-95736388

[elmart]

Guys, I think this is degenerating into a kind of byzantine discussion. From all the above, I think we can consider that using if (p) is perfectly safe.

[ZyX-I]

@elmart It is a discussion primary about coding style. We all know that “any C compiler which defines NULL to anything other than 0 is guaranteed to not run any useful software”, but if I have an additional point in defence to my position (pointer checks should be explicit) I will propagate it because primary point (“it is less readable”, expanded in one of the above comments (not mine)) is rather subjective.

[justinmk]

For @returns in doxygen comments, note [out] and [allocated] conventions.

https://github.com/neovim/neovim/pull/2470#discussion_r29104518

[justinmk]

Also need a caveat in the comment style recommendation regarding macro comments: https://github.com/neovim/neovim/pull/2578#discussion_r29609134

[dundargoc]

@justinmk Huh, I guess this is still relevant since the VLA section hasn't been updated. I'll create a PR now as to not forget it.

Is there anything else of use here to us? Skimmed through it quickly but mostly felt like I got a lecture on null pointers.

[justinmk]

@dundargoc i think this would be worth adding to :help dev-style:

For @param and @returns in doxygen comments, note the [in], [out], [allocated] conventions.

[dundargoc]

Plan to add it in this PR.

[justinmk]

Style is now fully defined in the neovim/neovim repo in src/uncrustify.cfg and :help dev-style