Changeset View
Standalone View
src/script/script.h
Show First 20 Lines • Show All 267 Lines • ▼ Show 20 Lines | public: | ||||
} | } | ||||
inline CScriptNum operator+(const CScriptNum &rhs) const { | inline CScriptNum operator+(const CScriptNum &rhs) const { | ||||
return operator+(rhs.m_value); | return operator+(rhs.m_value); | ||||
} | } | ||||
inline CScriptNum operator-(const CScriptNum &rhs) const { | inline CScriptNum operator-(const CScriptNum &rhs) const { | ||||
return operator-(rhs.m_value); | return operator-(rhs.m_value); | ||||
} | } | ||||
inline CScriptNum operator/(const int64_t &rhs) const { | |||||
return CScriptNum(m_value / rhs); | |||||
schancel: It would be nice if division by zero was in here, but I think throwing an exception would be… | |||||
shaddersUnsubmitted Not Done Inline ActionsThe div-by-zero check is in intepreter.cpp so in theory a zero should never be passed to this function. But that doesn't preclude the possibility of a future op code calling the div operator without a zero check. Given that in theory the exception would never be triggered unless someone makes a mistake in the future would an exception here to act as a guard be a problem? I don't know if C++ has the concept of checked and unchecked exceptions but if so I would suggest an unchecked exception. Whatever happens here should happen to the mod operator as well. shadders: The div-by-zero check is in intepreter.cpp so in theory a zero should never be passed to this… | |||||
jasonbcoxUnsubmitted Not Done Inline ActionsChanging the implementation to: return m_value / CScriptNum(rhs); would call what I'd consider to be the "base" operator that's implemented below. Funneling calls into the same codepaths is generally good design as it discourages re-implementation errors. (for example, if you add a divide-by-zero check, you only need to add it to the division operator below). jasonbcox: Changing the implementation to:
return m_value / CScriptNum(rhs);
would call what I'd… | |||||
jasonbcoxUnsubmitted Not Done Inline ActionsOops I had it backwards, the below operator actually calls this one. Nevermind! jasonbcox: Oops I had it backwards, the below operator actually calls this one. Nevermind! | |||||
schancelAuthorUnsubmitted Not Done Inline ActionsAn exception could be equally as bad. Is an std::optional or std::variant a reasonable possibility? Paging @deadalnix for an opinion. schancel: An exception could be equally as bad. Is an `std::optional` or `std::variant` a reasonable… | |||||
danconnollyUnsubmitted Not Done Inline ActionsGentle repaging @deadalnix for an opinion on whether and how to add another level of checking for div by zero here, in case the operator is used by some other code in the future. danconnolly: Gentle repaging @deadalnix for an opinion on whether and how to add another level of checking… | |||||
} | |||||
inline CScriptNum operator/(const CScriptNum &rhs) const { | |||||
return operator/(rhs.m_value); | |||||
} | |||||
inline CScriptNum &operator+=(const CScriptNum &rhs) { | inline CScriptNum &operator+=(const CScriptNum &rhs) { | ||||
return operator+=(rhs.m_value); | return operator+=(rhs.m_value); | ||||
} | } | ||||
inline CScriptNum &operator-=(const CScriptNum &rhs) { | inline CScriptNum &operator-=(const CScriptNum &rhs) { | ||||
return operator-=(rhs.m_value); | return operator-=(rhs.m_value); | ||||
} | } | ||||
inline CScriptNum operator&(const int64_t &rhs) const { | inline CScriptNum operator&(const int64_t &rhs) const { | ||||
▲ Show 20 Lines • Show All 366 Lines • Show Last 20 Lines |
It would be nice if division by zero was in here, but I think throwing an exception would be bad.