olimasters / magnet-automaton

Visualised automaton-based simulation of a magnet
1 stars 0 forks source link

-Wsequence-point raises warning about potential undefined behaviour #8

Closed olimasters closed 7 years ago

olimasters commented 7 years ago

54:45 of Magnet.cpp. This should either be refactored, or it should be scrutinised to make sure that it is in fact defined behaviour.

AlexSWall commented 7 years ago

I don't know what -Wsequence-point is, but I've double-checked and it looks like, assuming i,j are valid indices of the array, behaviour will all be defined.

olimasters commented 7 years ago

-Wsequence-point is one of the flags given to gcc by the -Wall flag. The issue is that behaviour is undefined in some cases when the same variable is changed by more than one operation with side effects in the same expression. For example, I think that

int i = 0;
i = i++ + i++;

is undefined behaviour. However, in this case, due to the presence of the && operators, I think we are okay. See https://en.wikipedia.org/wiki/Sequence_point for some more reading on the matter

olimasters commented 7 years ago

In fact, even i = i++ is undefined behaviour.

AlexSWall commented 7 years ago

Riight... perhaps. I am not certain in any way.

olimasters commented 7 years ago

This repository is a Java-free zone I'm afraid

AlexSWall commented 7 years ago

I believe we are guaranteed the increment to the variable 'neighbours' necessarily occurs before checking its boolean state, and no other assignment occurs (on the right hand side), so I don't think there is a problem.

olimasters commented 7 years ago

True. I might end up changing it just out of being annoyed by the compiler warning though.

AlexSWall commented 7 years ago

Change it how? I assume you're not going to change it to the way I was thinking of doing it?