Path: ...!2.eu.feeder.erje.net!feeder.erje.net!eternal-september.org!feeder3.eternal-september.org!news.eternal-september.org!.POSTED!not-for-mail
From: David Brown <david.brown@hesbynett.no>
Newsgroups: comp.lang.c
Subject: Re: Code guidelines
Date: Wed, 4 Sep 2024 20:14:42 +0200
Organization: A noiseless patient Spider
Lines: 172
Message-ID: <vba82i$3u4jc$2@dont-email.me>
References: <vb6v1t$3b5mb$1@dont-email.me> <vb726n$3b4rq$1@dont-email.me>
 <vb736j$3b5mb$2@dont-email.me> <vb75g9$3bntp$1@dont-email.me>
 <vb77tn$3bu07$3@dont-email.me> <vb7d6l$3d5mv$1@dont-email.me>
 <vb7dve$3d5mv$2@dont-email.me> <vb8vlv$3nlvo$1@dont-email.me>
 <vb9hlv$3qa2u$1@dont-email.me>
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 8bit
Injection-Date: Wed, 04 Sep 2024 20:14:43 +0200 (CEST)
Injection-Info: dont-email.me; posting-host="007bf4bb57fea4fb73ad9dc6d5dccf66";
	logging-data="4133484"; mail-complaints-to="abuse@eternal-september.org";	posting-account="U2FsdGVkX1/IOO/9W/Xs8FNvC0+SHb2BNurOcg+rhZ4="
User-Agent: Mozilla Thunderbird
Cancel-Lock: sha1:GG6RfN4fV5wunAMgPKXiHLL6mHo=
Content-Language: en-GB
In-Reply-To: <vb9hlv$3qa2u$1@dont-email.me>
Bytes: 7048

On 04/09/2024 13:52, Thiago Adams wrote:
> On 04/09/2024 03:45, David Brown wrote:
>> On 03/09/2024 18:37, Thiago Adams wrote:
>>
>>>
>>> I also have a interesting sample with linked list
>>>
>>
>> Speaking generally rather than specifically about this code, your 
>> extra checks are redundant when the code is run from a single thread, 
>> and insufficient if it is later used from multiple threads (accessing 
>> the same data).  But extra checks can fool people into thinking it is 
>> safe - since the checks are unnecessary for sequential code they must 
>> have been put there to catch rare race conditions.
>>
>>
>> There are times when you want seat-belts /and/ airbags.  But focusing 
>> on that instead of making sure the driver is competent is not helpful!
>>
>>
>>
> 
> 
> 
> In Cake (my front-end), I made `assert` a special built-in function to 
> allow it to be used by static analysis.
> 
> For instance:
> 
> `assert(p != 0);`
> 
> Static analysis will then assume that `p` is not zero after this line.

It will (or at least, it can), if the assert is enabled.  If NDEBUG is 
defined then the standard assert macro does nothing - it does not tell 
the compiler anything.  And compilers - baring bugs, as always - do not 
optimise by guessing assumptions.

> 
> However, this raises a question:
> Does assert overrides what the compiler already knows?

If you tell the compiler two contradictory things, you can't expect good 
results.  It is reasonable for it to believe the first thing you said, 
or the second, or neither, or both at different times.  Ideally, of 
course, it will give a warning.

Do not lie to your compiler.  It will result in tears - /your/ tears, 
not the compiler's.

> 
> For example:
> 
> int i = 1;
> assert(i == 2);
> 
> Here, the compiler knows `i` is `1`, but the `assert` (i.e., the 
> programmer) is claiming that it’s `2`!
> 

You can't expect good results from nonsense - at best, you can hope for 
a warning.

This is one reason why I use my own macro:

extern void __attribute__((error("Assume failed"))) assumeFailed(void);
#define Assume(x) \
         do { \
                 if (__builtin_constant_p(x)) { \
                         if (!(x)) { \
                                 assumeFailed(); \
                         } \
                 } \
                 if (!(x)) __builtin_unreachable(); \
         } while (0)

(It's actually a bit more complicated since, like standard assert(), it 
supports enabling or disabling.  But that's the key part.)

This will catch the mistake in your example, if you use "Assume" instead 
of "assert".


> I then reconsidered my view. `Assert` is about what the programmer is 
> stating to be true. (Programmer can be wrong) It is not a "override 
> command".
> 
> I think this is applicable for any "assume concept".
> 
> C++ 23 added [[assume( expression )]]
> 
> "Tells the compiler to assume that an expression will always evaluate to 
> true at a given point ..." "The purpose of an assumption is to allow 
> compiler optimizations based on the information given"
> 
> There is a big warning there :
> "IMPORTANT: DANGEROUS OPERATION, USE WITH CARE"
> 
> https://en.cppreference.com/w/cpp/language/attributes/assume

"[[assume(E)]];" in C++23, or "__attribute__((assume(E)));" as an 
extension in newer gcc in any C or C++ standard, is basically the same 
as "if (!(E)) __builtin_unreachable();".

My "Assume" macro is better in most cases, because it catches errors 
that are seen at compile time, while the assume attribute would 
eliminate code with no warning.

> 
> I also may add this [[assume]] to cake, but assert also gives me a 
> runtime check.
> 
> In the first example `assert(p != 0)`, the compiler knows that `p` is 
> MAYBE `null`. But the programmer is asserting that `p` cannot be `null`, 
> so the compiler will follow the programmer's assertion.
> 
> Perhaps the programmer knows something the compiler doesn’t?

If you want the best from your compiler (optimisations and error 
checking), tell it what you know.

> 
> This is similar to a case with a linked list, where the programmer knew 
> that if the `head` is `null`, then the `tail` must also be `null`.
> 
> However, if the compiler is certain about a fact and encounters an 
> `assert` with conflicting information, it should trigger a warning.
> 

An "assert" macro could be defined to do that, if you are happy to use 
the gcc extension "__builtin_constant_p()".  If not, then I don't think 
it is possible to specify that it should trigger a warning in any clear 
way unless the expression is a constant expression - and then you'd be 
using static_assert().

> For example:
> 
> int i = 0;
> assert(i != 0); // Warning
> 

So make a better "assert" that does what you want.

> 
> 
> When the compiler finds an assert, that is not adding extra information 
> but only saying what the compiler already knows, then we don't have 
> warnings. But, this is a redundant check.
> 
> int i = 0;
> assert(i == 0); // ok
> 
> On the other hand, redundant 'if' I have a warning.
> if (i == 0) {} //warning 'i' is always 0 here!
> 
> So, basically.
> - if assert has a conflicting information compared with what compiler 
> already knows, then it is a warning. Compiler will not override what it 
> knows?
> 
> - If the assert gives the exactly information compiler already have, 
> then is is not a warning. Both compiler and programmers knows the same 
> thing.
> 
> - If the compiler is not sure about a fact and the programmer gives a 
> plausible fact then the compiler will assume the programmer is correct.
> 
> 
> 
> 
>