[SERVER-29624] mongo::Status implicit conversion to mongo::ErrorCodes::Error Created: 14/Jun/17  Updated: 12/Feb/18  Resolved: 12/Feb/18

Status: Closed
Project: Core Server
Component/s: Build
Affects Version/s: None
Fix Version/s: None

Type: Improvement Priority: Minor - P4
Reporter: Nathan Myers Assignee: DO NOT USE - Backlog - Platform Team
Resolution: Won't Fix Votes: 0
Labels: None
Remaining Estimate: Not Specified
Time Spent: Not Specified
Original Estimate: Not Specified

Participants:

 Description   

If we add a conversion member to Status, we can use the idiom

   switch (auto status = fun()) {
        case ErrorCodes::Error::NoSuchKey: break;
        default: return status;
   }

For this to be safe, we would need to change Error to an "enum class". Otherwise, the implicit promotion ErrorCodes::Error -> int would allow a Status to be passed to a function taking int. That change would, in turn, require changing mentions of "ErrorCodes::<name>" to "ErrorCodes::Error::<name>", as seen above.

Alternatively, the type Error could be eliminated, and the enumerators promoted to an enum class ErrorCodes. That would require changing the ErrorCodes static members to namespace-scoped functions.

Or, the enum type Error could be left with no enumerators, but instead define constexpr values in ErrorCodes. That would not require changes in most uses of ErrorCodes, and the idiom would look like

   switch (auto status = fun()) {
        case ErrorCodes::NoSuchKey: break;
        default: return status;
   }



 Comments   
Comment by Andrew Morrow (Inactive) [ 30/Jun/17 ]

I'm marking this as 3.5 desired so that we don't lose track of it, but, more realistically, I think this is likely to be a post 3.8 project, for two reasons.

  • First, when such a thing has been proposed in the past, the ambiguity of resolving the implicit conversion has come up. Which form of Status is truthy - the error bearing one? Or the OK bearing one? Is there a convincing argument either way? How should it work with StatusWith? Does that inform the decision about pure Status?

if (auto swWithBSONObj = functionThatMightReturnABSONObj()) {
    // Seems like I want the value populated StatusWith to be truthy
}

Without C++17, we can't write:

if (auto swWithBSONObj = ...; swWithBSONObj) {
} else {
}

  • Second, I'd like to undertake an overall refactoring of how our ErrorCodes work. I want us to move towards using the C++11 error_category mechanism, or something similar. This would allow us to partition the error code space, such that different parts of the code could have their own enumeration of error values. This would get us out of the situation we are in now, where adding a new symbolic code of any sort requires more or less a complete rebuild of the tree, even if only a small subset cares about the new error.

I think we should defer this idea until we have a clearer sense of what semantics we want, that give us consistent behavior across Status, StatusWith, and C++17 init-in-control-flow changes.

Generated at Thu Feb 08 04:21:23 UTC 2024 using Jira 9.7.1#970001-sha1:2222b88b221c4928ef0de3161136cc90c8356a66.