On 03/09/2024 14:22, Thiago Adams wrote:

Witch style (F1, F2 ..) better represents the guidelines of your code?

//The function f never returns nullptr
int * f();

void F1() {
   int* p = f();

   if (p == nullptr) {
      assert(false);
      return;
   }
   // using *p ...
}

void F2()
{
   int* p = f();
   assert(p != nullptr);
   // using *p ...
}

void F3()
{
   int* p = f();
   // using *p ...
}

void F4()
{
   int* p = f();
   if (p) {
    // using *p ...
   }
}

My preferred style would be :

int * f() __attribute__((returns_nonnull));

Then F3 is appropriate.

(I would also have "(void)" in the declaration, unless I was using C23.)

The same question but for struct members

struct user {

  //Name is required, cannot be null.
  char * name;
};

void F1(struct user* user) {

   if (user->name == nullptr) {
      assert(false);
      return;
   }
   // using user->name ...
}

void F2(struct user* user) {
   assert(user->name != nullptr);
   // using user->name ...
}

void F3(struct user* user) {
   // using user->name ...
}

void F4(struct user* user) {
   if (user->name) {
     // using user->name ...
   }
}

void F5(struct user * user) {
if (!user->name) __builtin_unreachable();
// using user->name;
}

I am of the opinion that if something is clearly specified, you make
sure it is true when you are responsible for it - and then you can
assume it is true when you use the results. It makes no sense to me to
do something, and then immediately check that it is done.

Whenever possible, these specifications should be in code, not comments
- using whatever tools and extra features you have available for use.
Macros and conditional compilation help make code more portable, and can
also let you turn compile-time assumptions into run-time checks during debugging.

--- SoupGate-Win32 v1.05
* Origin: fsxNet Usenet Gateway (21:1/5)

On 03/09/2024 15:33, Thiago Adams wrote:

On 03/09/2024 10:16, David Brown wrote:

On 03/09/2024 14:22, Thiago Adams wrote:

I am of the opinion that if something is clearly specified, you make
sure it is true when you are responsible for it - and then you can
assume it is true when you use the results.  It makes no sense to me
to do something, and then immediately check that it is done.

Whenever possible, these specifications should be in code, not
comments - using whatever tools and extra features you have available
for use. Macros and conditional compilation help make code more
portable, and can also let you turn compile-time assumptions into
run-time checks during debugging.

Yes, this contract "function don't returns null" is very straightforward
for the caller and for the implementer.

The contract implementation can be checked inside function
implementation. (One place to check the contract implementation)
The contract usage, is checked in each caller, but very straightforward.

On the other hand, struct members have much more places where the
contract can be broken. Each function that have a non const access to user->name could break the contract implementation.
The risk is much bigger compared with the return case.

You are asking about "guidelines". Yes, it is possible for code to
break specifications. The guideline is "don't do that".

If you are mixing untrusted code with trusted code, then you have to
check the incoming data just like you do with data from external
sources. But most functions are not at such boundaries.

So I think it may make sense to have redundant runtime checks, but it
must be clear the the "compile time contract" is not that one.

For instance:

The first sample my create confusion (is name optional?)

void f(struct user* user)
{
     if (user->name && strcmp(user->name, "john") == 0)
     {
        //...
     }
 }

But :
void f(struct user* user)
{
     assert(user->name);
     if (user->name && strcmp(user->name, "john") == 0)
     {
        //...
     }
 }

would show redundancy but making clear the contract still "name should
not be null"

No, redundant runtime checks are not helpful. If they are redundant,
they are by definition a waste of effort - either run-time efficiency,
or developer efficiency, or both. And they are a maintenance mess as
changes have to be applied in multiple places.


void f(struct user* user)
{
if (!user->name) __builtin_unreachable();
if (strcmp(user->name, "john") == 0) {
//...
}
}

Now it is /really/ clear that user->name should not be null, and instead
of two run-time checks doing the same thing, there are no run-time costs
and the compiler may even be able to optimise more from the knowledge
that the pointer is not null.

In practice I use a macro called "Assume" here, which will give a hard compile-time error if the compiler can that the assumption is false. It
also supports optional run-time checks depending on a #define'd flag
(normally set as a -D compiler flag), as an aid to debugging.

Write things clearly, once, in code.

If the code is boundary code, then you need a check - but then a simple
null pointer check is unlikely to be sufficient. (What if the pointer
is not null, but not a valid pointer? Or it does not point to a string
of a suitable length, with suitable character set, etc.?)

--- SoupGate-Win32 v1.05
* Origin: fsxNet Usenet Gateway (21:1/5)

On 03/09/2024 16:37, Thiago Adams wrote:

On 03/09/2024 11:10, David Brown wrote:

On 03/09/2024 15:33, Thiago Adams wrote:

On 03/09/2024 10:16, David Brown wrote:

On 03/09/2024 14:22, Thiago Adams wrote:

I am of the opinion that if something is clearly specified, you make
sure it is true when you are responsible for it - and then you can
assume it is true when you use the results.  It makes no sense to me
to do something, and then immediately check that it is done.

Whenever possible, these specifications should be in code, not
comments - using whatever tools and extra features you have
available for use. Macros and conditional compilation help make code
more portable, and can also let you turn compile-time assumptions
into run-time checks during debugging.

Yes, this contract "function don't returns null" is very
straightforward for the caller and for the implementer.

The contract implementation can be checked inside function
implementation. (One place to check the contract implementation)
The contract usage, is checked in each caller, but very straightforward. >>>
On the other hand, struct members have much more places where the
contract can be broken. Each function that have a non const access to
user->name could break the contract implementation.
The risk is much bigger compared with the return case.

You are asking about "guidelines".  Yes, it is possible for code to
break specifications.  The guideline is "don't do that".

The guidelines are more about the usage of runtime checks, are they
required? optional? etc.. when to use etc...

The guideline is "follow the specifications". Then it is obvious that
run-time checks are not required unless you are dealing with code that
does not follow the specifications.

Now, as you note below, code under development might not follow the specifications - that is why it is useful to do this sort of thing with
macros that support adding run-time checks during development or
debugging. Standard C "assert" is a poor man's version of this.

If you are mixing untrusted code with trusted code, then you have to
check the incoming data just like you do with data from external
sources.  But most functions are not at such boundaries.

yes ..agree.

So I think it may make sense to have redundant runtime checks, but it
must be clear the the "compile time contract" is not that one.


For instance:

The first sample my create confusion (is name optional?)

void f(struct user* user)
{
      if (user->name && strcmp(user->name, "john") == 0)
      {
         //...
      }
  }

But :
void f(struct user* user)
{
      assert(user->name);
      if (user->name && strcmp(user->name, "john") == 0)
      {
         //...
      }
  }

would show redundancy but making clear the contract still "name
should not be null"

No, redundant runtime checks are not helpful.  If they are redundant,
they are by definition a waste of effort - either run-time efficiency,
or developer efficiency, or both.  And they are a maintenance mess as
changes have to be applied in multiple places.

But we know , when the code is new (under development) the contract may
not be so clear or change very frequently.

Then it is even worse to have redundant checks - when they are
inconsistent, you have no idea what is going on or what /should/ be
going on.

     void f(struct user* user)
     {
         if (!user->name) __builtin_unreachable();
         if (strcmp(user->name, "john") == 0) {
             //...
         }
     }

Now it is /really/ clear that user->name should not be null, and
instead of two run-time checks doing the same thing, there are no
run-time costs and the compiler may even be able to optimise more from
the knowledge that the pointer is not null.

In practice I use a macro called "Assume" here, which will give a hard
compile-time error if the compiler can that the assumption is false.
It also supports optional run-time checks depending on a #define'd
flag (normally set as a -D compiler flag), as an aid to debugging.

Write things clearly, once, in code.

If the code is boundary code, then you need a check - but then a
simple null pointer check is unlikely to be sufficient.  (What if the
pointer is not null, but not a valid pointer?  Or it does not point to
a string of a suitable length, with suitable character set, etc.?)

The macro I need is something like

"I know this check is redundant, but I will add it anyway" "this check
is redundant but according with the contract"

If a check is redundant, and the compiler can see that, the extra check
will be eliminated by optimisation. So you are wasting developer time
for nothing.

assert/assume is more

"trust me compiler, this is what happens" someone else is checking this
we don't need to check again.

That is not what "assert" is, but it is roughly what my "Assume" is.
And yes, that is what you should be aiming for in the code.

--- SoupGate-Win32 v1.05
* Origin: fsxNet Usenet Gateway (21:1/5)

On 03/09/2024 16:12, Thiago Adams wrote:

On 03/09/2024 10:33, Thiago Adams wrote:
...

For instance:

The first sample my create confusion (is name optional?)

void f(struct user* user)
{
      if (user->name && strcmp(user->name, "john") == 0)
      {
         //...
      }
  }

But :
void f(struct user* user)
{
      assert(user->name);
      if (user->name && strcmp(user->name, "john") == 0)
      {
         //...
      }
  }

would show redundancy but making clear the contract still "name should
not be null"

Redundant code can either indicate a programmer's mental confusion

Yes.

or
serve as a way to address potential contract violations.

No.

If specification violations are realistic (from untrusted code, or code
under development), then a /single/ check looks for violations.
/Redundant/ checks are pointless at best, and (as I have explained)
often worse than useless.

Computers are not humans that might miss something on the first glance,
then see it on the second time. Do the same check twice in the code and
you will get the same answer each time - the second check gives no benefits.

I believe the objective is to ensure that runtime checks are not
questioning the contract but rather functioning as redundant safeguards.

In other words, the programmer must demonstrate that they understand the contract and are not messing it.

A safeguards for a very low risk situation also may indicate a mental confusion about the risks involved. For instance, assert(2 + 2 == 4);

A redundant check is, by definition, a very low risk situation.

--- SoupGate-Win32 v1.05
* Origin: fsxNet Usenet Gateway (21:1/5)

On 2024-09-03, Thiago Adams <thiago.adams@gmail.com> wrote:

Witch style (F1, F2 ..) better represents the guidelines of your code?

//The function f never returns nullptr
int * f();

^^^^^^

Nope.

void F1() {

int* p = f();

^^^^

Nope.

if (p == nullptr) {
assert(false);
return;
}
// using *p ...
}

void F2()
{
int* p = f();

^^^

Nope.

assert(p != nullptr);
// using *p ...
}

void F3()
{
int* p = f();

^^^^

Nope.

// using *p ...
}

void F4()
{
int* p = f();

And nope.

The syntax of a declaration is, roughly, "specifier-list comma-separated-declarators".

"int" is the specifier list, "*p" is the declarator.

Your style invites optical illusions like:

int* a, b; // two pointers?

int *a, b; // Nope: actually this.

In C, declarators are type-constructing expressions which (at least in
their basic, unadorned forms) resemble how the name will be used.

For instance when you see "int a[3], *p", you can
interpret it as saying that "the expressions a[3] and *p
shall have type int".


--
TXR Programming Language: http://nongnu.org/txr
Cygnal: Cygwin Native Application Library: http://kylheku.com/cygnal
Mastodon: @Kazinator@mstdn.ca

--- SoupGate-Win32 v1.05
* Origin: fsxNet Usenet Gateway (21:1/5)

F2 in both cases.



--
Blue-Maned_Hawk│shortens to Hawk│/blu.mɛin.dʰak/│he/him/his/himself/Mr. blue-maned_hawk.srht.site

--- SoupGate-Win32 v1.05
* Origin: fsxNet Usenet Gateway (21:1/5)

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!

--- SoupGate-Win32 v1.05
* Origin: fsxNet Usenet Gateway (21:1/5)

On 03/09/2024 18:23, Thiago Adams wrote:

On 03/09/2024 11:53, David Brown wrote:

On 03/09/2024 16:12, Thiago Adams wrote:

A safeguards for a very low risk situation also may indicate a mental
confusion about the risks involved. For instance, assert(2 + 2 == 4);

A redundant check is, by definition, a very low risk situation.

I will give a sample

<snip>

but...maybe, is better to be a little redundant here?
I think I prefer to leave "obj->member1->member2 && " even if I know
it should not be null.

if (obj->member1 &&
obj->member1->member2 &&
obj->member1->member2->member3)
{

}

No.

Well, there might occasionally be times where a pattern in the code
layout gives so much extra clarity or increases maintainability so much
that the redundancy is worth it.

Instead of thinking that the risk of someone breaking the specifications
is so high that it is worth putting in extra checks, deal with the
/real/ problem - ask why there is that risk. Is the person writing the
rest of the code (you, or someone else) capable of the task or do they
need help, guidance, extra training, etc.? Are the specifications
unclear or in an inconvenient place? Are the specifications
particularly onerous or illogical?

Before you put any check in code, think about the circumstances in which
it could fail. If there are no circumstances, it is redundant and counter-productive. If it /could/ fail, think about /why/ it could fail
- can you do something to ensure the failure will not occur? Then fix
the source of the problem, and the check is redundant. Think about what
you can do with the information about the failure - can you recover?
Can you mitigate damage? Can you at least report the issue in a useful
way? If there's nothing you can do, why check it?

This kind of obsessive "check each pointer for null before using it" is
a bad habit. It is arse-covering of the worst kind - it's about being
able to claim "it's not /my/ fault the program doesn't work". It is not
a real check of pointer validity, or being sure the code works as it
should - it's just a show to make it look like you take code quality
seriously. And in almost all cases, it is never tested - no one ever
checks through all the combinations of possible failures, and the
cyclomatic complexity of the code quickly makes such tests impractical.

Focus on how to avoid code failing to match the specifications - not on pointless redundant checks. Get in the habit of checking for /real/
problems at run-time. If you keep putting in these extra checks without thinking, you'll start believing your code is safe and has all the
checks it needs - and miss out on the things that are important.

--- SoupGate-Win32 v1.05
* Origin: fsxNet Usenet Gateway (21:1/5)

On 04/09/2024 09:22, Keith Thompson wrote:

David Brown <david.brown@hesbynett.no> writes:
[...]

Before you put any check in code, think about the circumstances in
which it could fail. If there are no circumstances, it is redundant
and counter-productive.

[...]

One thing to consider is that if a check can never actually fail the
recovery code *cannot be tested* (and you can't get 100% code coverage).

p = NULL; // assume p is not volatile
if (p != NULL) {
do_something(); // can never execute this
}

Of course not all such cases are so easily detectible (

I wrote "in almost all cases, it is never tested" - but as you say, in
some cases it /cannot/ ever be tested because the test conditions can
never be triggered.

I think, however, that "could be tested, but is not tested" is worse.
I've seen cases of code that has been put in for extra checks "just to
make sure" that had not been tested, and caused more trouble.

One case I remember was some extra checks for timeouts in some
communications. The new checks were unnecessary - a higher level
timeout mechanism was already in place, tested, and working. The new
checks were never tested, and never triggered during normal operation.
But when a 32-bit millisecond counter rolled over, the check was wrong triggered - and the handling code was buggy and hung. Thus the
unnecessary and untested extra check resulted in systems hanging every
49 days.

--- SoupGate-Win32 v1.05
* Origin: fsxNet Usenet Gateway (21:1/5)

On 2024-09-03, Thiago Adams <thiago.adams@gmail.com> wrote:

The contract is

* obj->member1 CAN be null
* obj->member1->member2 CANNOT be null
* obj->member1->member2->member3 CAN be null

Newer languages have null-safe object access operators:

if (obj?->member1->member2?->member3) ...

We get the correct check, and, at a glance, the question marks annotate
what the coder believes is the contract: obj may be null,
obj->member1->member2 may be null.

GCC could easily get this extension, it seems.

obj?->member is just obj ? obj->member : nullptr except that
obj is evaluated only once.


--
TXR Programming Language: http://nongnu.org/txr
Cygnal: Cygwin Native Application Library: http://kylheku.com/cygnal
Mastodon: @Kazinator@mstdn.ca

--- SoupGate-Win32 v1.05
* Origin: fsxNet Usenet Gateway (21:1/5)

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.

--- SoupGate-Win32 v1.05
* Origin: fsxNet Usenet Gateway (21:1/5)

On 04/09/2024 16:02, Kaz Kylheku wrote:

On 2024-09-03, Thiago Adams <thiago.adams@gmail.com> wrote:

The contract is

* obj->member1 CAN be null
* obj->member1->member2 CANNOT be null
* obj->member1->member2->member3 CAN be null

Newer languages have null-safe object access operators:

if (obj?->member1->member2?->member3) ...

We get the correct check, and, at a glance, the question marks annotate
what the coder believes is the contract: obj may be null, obj->member1->member2 may be null.

GCC could easily get this extension, it seems.

obj?->member is just obj ? obj->member : nullptr except that
obj is evaluated only once.

gcc is not as keen on making new extension syntax as it was some 30 year
ago. They try to keep extensions to a minimum, or fitting them into
existing patterns (__builtin functions and __attribute__'s). Or they
import new features from later C or C++ standards back to older standard versions.

And they tend to have the attitude that programmers who want a language
that does more than current C, will probably use a different language.
In C++ you can often specify non-null requirements using references
rather than pointers, and it's not hard to make a little class template
that act mostly like a pointer except that it checks for null first and
throws an exception on null. (Making something that will chain like
your code, resulting in a bool true if everything is good to go, is I
believe possible but a bit more fiddly. Ask for details down the hall.)

--- SoupGate-Win32 v1.05
* Origin: fsxNet Usenet Gateway (21:1/5)

On Tue, 3 Sep 2024 11:37:16 -0300, Thiago Adams wrote:

assert/assume is more

"trust me compiler, this is what happens" someone else is checking this
we don't need to check again.

Alphard made a clear distinction between “assert” and “assume”.

--- SoupGate-Win32 v1.05
* Origin: fsxNet Usenet Gateway (21:1/5)

On 03/09/2024 17:23, Thiago Adams wrote:
<snip>

but...maybe, is better to be a little redundant here?
I think I prefer to leave "obj->member1->member2 && " even if I know
it should not be null.

if (obj->member1 &&
    obj->member1->member2 &&
    obj->member1->member2->member3)
{

}

I think I'd prefer to _omit_ the check that obj->member1->member2 isn't
null.

If the code is running correctly that check will never trigger, and is redundant, and will slow things slightly.

But if the code is not running correctly it will conceal the bug by
preventing the crash.

But circumstances may vary. You probably don't want to crash an
autopilot if at all possible!

Andy

--- SoupGate-Win32 v1.05
* Origin: fsxNet Usenet Gateway (21:1/5)

On 2024-10-01, Vir Campestris <vir.campestris@invalid.invalid> wrote:

On 03/09/2024 17:23, Thiago Adams wrote:
<snip>

but...maybe, is better to be a little redundant here?
I think I prefer to leave "obj->member1->member2 && " even if I know
it should not be null.

if (obj->member1 &&
    obj->member1->member2 &&
    obj->member1->member2->member3)
{

}

I think I'd prefer to _omit_ the check that obj->member1->member2 isn't
null.

If the code is running correctly that check will never trigger, and is redundant, and will slow things slightly.

How about:

#define vassert(x) (assert(x), 1)

if (obj->member1 &&
vassert(obj->member1->member2) &&
obj->member1->member2->member3)

If NDEBUG is defined, that just reduces to 1, and is optimized
away. If NDEBUG is absent, the null pointer crash is replaced
by an assertion failure.

--
TXR Programming Language: http://nongnu.org/txr
Cygnal: Cygwin Native Application Library: http://kylheku.com/cygnal
Mastodon: @Kazinator@mstdn.ca

--- SoupGate-Win32 v1.05
* Origin: fsxNet Usenet Gateway (21:1/5)