squirrel: sqvm: remove useless assert(0) to fix GCC6 warning

[M1cha]

this is the warning:
squirrel/sqvm.cpp: In member function 'bool SQVM::Execute(SQObjectPtr&, SQInteger, SQInteger, SQObjectPtr&, SQBool, SQVM::ExecutionType)':
squirrel/sqvm.cpp:1096:1: error: control reaches end of non-void function [-Werror=return-type]
}
^
cc1plus: all warnings being treated as errors

the assert(0) is useless because there's an unavoidable "return false" above.
this patch is related to the following GCC bug:
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=71943

[zeromus]

Looks good to me.

[iSLC]

This bugged me even with lower versions of GCC/MinGW and not just GCC6. The assert can even be left there if necessary, but something must be returned after it.

[MrSapps]

Isn't the point of this to assert you never reach this point? So removing it isn't a good idea.

[zeromus]

Surely someone else's compiler settings will alert that you can reach the end of a non-void function truthfully--the assert is only for a compiler which isn't detecting that. It would gall me to change my code to work around a compiler bug though which the user has chosen to turn into an error. (Although if you ask me, reaching the end of a non-void function should be universally an error)

[M1cha]

@zeromus is it possible that you're using "non-void function" wrong? the return-type of a function has nothing to do with it's termination.

[zeromus]

Yes it does have to do with its termination. What's returned if the end of a non-void function is reached? Undefined. Sometimes a warning, sometimes an error, sometimes garbage.

[M1cha]

what do you mean by "the end of a non-void function is reached?". In case you mean reaching the end without returning then yes, the value of the return register is undefined in this case.

The problem is that with this function there's no way that it skips the return statement.
Don't get confused by the { ... } block, it's just a useless context block without any condition. it's always entered.

[zeromus]

I know that. We all know that. The problem here is that gcc has a bug where it doesn't know that, and it prints a spurious warning, and some people turn warnings into errors, and this patch tries to make the spurious error go away by making an edit which doesnt even follow from the error message, deleting code which should be benign, and in fact helpful, but is only helpful in the case where no other compilers warn when or error when the function end is TRULY reachable without having a value returned. You're arguing with something I didn't even write to you, I wrote it to paulsapps. I already responded to you by saying "looks good to me" which I said because in fact this code currently can't reach the end of the non-void function without returning a value. I doubled down on this assessment by saying that the main reason for leaving the assert--protecting against the non-void-end-reached mistake--was a weak reason since someone's compiler would detect it.

I hope this recap of the entire thread helps you understand why you're not understanding me.

[MrSapps]

huh curious, I would have expected an unreachable code warning from MSVC in this case, since its effectively return false; assert(0);

[zeromus]

msvc seems to only check that when optimizing. But ordinarily that would mean switch to release mode and have the assert go to a nop, so you wouldnt get it there. It can catch the condition in principle in release mode though.

[M1cha]

I don't know if this still makes sense or not but given that it's been 6 years, I'm okay with closing this to remove it from my PR list 😛