Make return-type a compilation error #8362
Conversation
This protects from undefined behavior cases that can be added to the code by mistake.
| @@ -1124,6 +1124,7 @@ namespace | |||
| return twelveHours == 12 ? twelveHours : 12 + twelveHours; | |||
|
|
|||
| cb->err(Arg::Gds(isc_incorrect_hours_period) << string(period.data(), period.length())); | |||
There was a problem hiding this comment.
I wonder if this line can do anything but throw. If no it would be better to mark it as noreturn and undo addition of return.
There was a problem hiding this comment.
Normally it should throw. But we still can't be 100% sure. If we add noreturn and cb->err won't throw, the server will crash. I prefer to avoid such cases as well.
There was a problem hiding this comment.
I tend to agree with @aafemt - err function always throws, that's unconditional behavior.
But there is another side of a question. My old hack with passing different error raising calls was (among others) added to support descriptors in qli, at the same time avoid calls (like MOB_/CVT_) from qli directly to jrd and keep correct diags in engine. Currently, when qli is gone away, ideally that hack should better be rolled back. After that we will get here [[ noreturn ]] function in usual way.
I.e. please mark err function as [[ noreturn ]], it will be cleaned out together with both cb and err in it. IMO further deep modifications of this code make no sense.
There was a problem hiding this comment.
I.e. please mark err function as [[ noreturn ]], it will be cleaned out together with both cb and err in it. IMO further deep modifications of this code make no sense.
BTW, I have no idea how to mark err as noreturn while it's a function pointer.
This protects from undefined behavior cases that can be added to the code by mistake.