I have been re-writing large chunks of Vrui recently, primarily to support a new Vulkan-based HMD graphics driver that will warp and distortion-correct rendered application image frames to an HMD operating in “direct mode,” i.e., without being managed by the window manager. Yes, I know I’m several years late to that particular party. 🙂

While I was doing that, which involved staring at a lot of old code for extended periods, I also cleaned up some things that had been bugging me for a long time. Specifically, error handling. I like descriptive error messages, because I find they make it easier to pin-point problems encountered by users of my software who are not themselves programmers, like, say, people who install an AR Sandbox at their location. I like it when an error message tells me what went wrong, and where it went wrong. Something like “I’m currently in method Z of class Y in namespace X, and I can’t open requested file A because of operating system error B.” In other words, I want error messages tagged with a location like “X::Y::Z,” and with parameters like a file name or OS error code. I also want to use exceptions, obviously. Unfortunately, C++’s standard exception classes don’t have methods to create exception objects with parameters, so, a very long time ago, I decided to roll my own.

While not exactly C++, I ended up implementing a system using variadic functions and printf-style format strings. For this particular use case, that seemed reasonably safe to do. This made it possible to throw exceptions with detailed error messages in a single line of code, which I find crucial for readability in functions where a lot of things can go wrong, because otherwise error handling would overwhelm the actual functionality. As it is, there are currently hundreds of lines of code (956, to be precise) in Vrui that look like this:

Misc::throwStdErr("Vrui::SceneGraphManager::loadSceneGraph: Unable to open scene graph file %s due to error %d (%s)",fileName.c_str(),errno,strerror(errno));

Immediately, I see three problems with that idiom:

1. strerror is not thread safe. Oops. Its thread-safe replacements need to be provided with a buffer, which requires an extra line of code and makes the call to strerror_r overly long.
2. Misc::throwStdErr throws an exception from inside the function, meaning the compiler doesn’t know that the function never returns, which leads to spurious warnings, potentially bad optimization, etc.
3. I’ll have to type things like “Vrui::SceneGraphManager::loadSceneGraph” a lot, and all of those strings end up as constant data in the object/library files, and that just annoys me.

To fix point 1, we need to hide the strerror() call inside some new function like Misc::throwLibcErr() that takes errno as a parameter and does the rest behind the scenes. We can then extend that same mechanism to other classes of errors with predefined parameters, like Vulkan API errors.

To fix point 2, we need to return a std::runtime_error object and throw that from the calling location, instead of throwing from inside the called function. I didn’t do that initially because it seemed wasteful at the time, but that was a severe case of premature optimization, given that these things only happen when something already went seriously wrong, so there’s no point in being too clever about it.

To fix point 3, we need to find some way of using knowledge the compiler already has, specifically the name of the function it is currently compiling, and generate a location automatically.

__PRETTY_FUNCTION__

Let’s talk about point 3 first. g++ offers a pseudo-macro __PRETTY_FUNCTION__ that returns the (very!) detailed name of the current function as a C string. This is a great start, but for practical purposes, the name it returns is way too detailed. For example, here is its result for some class method I just made up:

static Geometry::Point (* Foo::Bar::makeAdder())(const Geometry::Point&, const Geometry::Vector&, ScalarParam) [with ScalarParam = float; AddFunction = Geometry::Point (*)(const Geometry::Point&, const Geometry::Vector&, float)]

We sure don’t want any end users to have to type that into an email. 😉 We only want “Foo::Bar::makeAdder.” Fortunately, this is pretty straightforward to achieve. We make a function Misc::parsePrettyFunction that takes the output of __PRETTY_FUNCTION__ and returns only the function name, including namespace and class prefixes. While we’re at it, we’re also going to remove any template parameters occurring in the function name. Those might be useful and I might put them back in later, but for now they’re gone. Here is the code for Misc::parsePrettyFunction:

std::string parsePrettyFunction(const char* prettyFunction)
  {
  /* Find the start and end of the function name by looking for the argument list's opening parenthesis, but ignore "(*" indicating a function-type result: */
  const char* fnStart=prettyFunction;
  const char* fnEnd;
  for(fnEnd=prettyFunction;fnEnd!='\0'&&(fnEnd!='('||fnEnd[1]=='*');++fnEnd)
    if(*fnEnd==' ')
      fnStart=fnEnd+1;
    
  /* Copy the function name into a new string while skipping any template parameters: */
  std::string result;
  unsigned int templateLevel=0;
  while(fnStart!=fnEnd)
    {
    if(*fnStart=='<')
      ++templateLevel;
    if(templateLevel==0)
      result.push_back(*fnStart);
    if(*fnStart=='>')
      --templateLevel;
    ++fnStart;
    }
  
  return result;
  }

I can see one thing wrong with this function — it mangles valid function names like operator<< or operator>= — but fixing that requires a lot more code and I promise I’ll do it later. 🙂 Apart from that, it works very well. It turns this:

static Geometry::Point (* Foo::Bar::makeAdder())(const Geometry::Point&, const Geometry::Vector&, ScalarParam) [with ScalarParam = float; AddFunction = Geometry::Point (*)(const Geometry::Point&, const Geometry::Vector&, float)]

… into this:

Foo::Bar::makeAdder

… and that’s really all I care about. Well, there is another problem. The function is brittle because it assumes a specific formatting of __PRETTY_FUNCTION__‘s result, which is not specified anywhere. Meaning, the next version of g++ is totally going to break this code, now that I’ve jinxed it. But at least it’s encapsulated in a single location, so it’ll be easy to fix when that inevitably happens.

A Better Throwing Idiom

Now let’s fix issue 2, which is straightforward. Instead of creating a std::runtime_error object inside Misc::throwStdErr and throwing it from right there, we return the created object and throw it from the calling location. This will clue in the compiler on what is going on. We make a new function Misc::stdErr:

std::runtime_error stdErr(const char* prettyFunction,const char* whatFormat,...)
  {
  /* Write everything into an on-stack buffer of sufficient size: */
  char whatBuffer[2048]; // If your error message is longer than this, you have other problems
  char* whatEnd=whatBuffer+sizeof(whatBuffer);
  char* whatPtr=whatBuffer;
  
  /* Shorten the __PRETTY_FUNCTION__ result and write it into the buffer, leaving room for a ": " separator and a NUL terminator: */
  whatPtr=parsePrettyFunction(prettyFunction,whatPtr,whatEnd-3); // Of course there's a buffer version of this function, what did you think?
  
  /* Add the separator: */
  *(whatPtr++)=':';
  *(whatPtr++)=' ';
  
  /* Print the formatted message into the buffer: */
  va_list ap;
  va_start(ap,whatFormat);
  vsnprintf(whatPtr,whatEnd-whatPtr,whatFormat,ap);
  va_end(ap);
  
  /* Create and return an exception object: */
  return std::runtime_error(whatBuffer);
  }

With this relatively simple function, we can now throw location-tagged and parametrized error messages like this:

throw Misc::stdErr(__PRETTY_FUNCTION__,"Cannot open file %s",fileName.c_str());

… which I have to admit I really like. And the compiler likes it better, too.

Classes Of Errors With Predefined Parameters

Finally, let’s sort out issue 1 by creating a new function Misc::libcError that works about the same, but takes errno as a parameter and appends a “due to error X (description of X)” suffix to the message. There’s going to be some code duplication because I need to deal with the message buffer directly multiple times and am too lazy to abstract this away:

std::runtime_error libcErr(const char* prettyFunction,int errno,const char* whatFormat,...)
  {
  /* Write everything into an on-stack buffer of sufficient size: */
  char whatBuffer[2048]; // If your error message is longer than this, you have other problems
  char* whatEnd=whatBuffer+sizeof(whatBuffer);
  char* whatPtr=whatBuffer;
  
  /* Shorten the __PRETTY_FUNCTION__ result and write it into the buffer, leaving room for a ": " separator and a NUL terminator: */
  whatPtr=parsePrettyFunction(prettyFunction,whatPtr,whatEnd-3); // Of course there's a buffer version of this function, what did you think?
  
  /* Add the separator: */
  *(whatPtr++)=':';
  *(whatPtr++)=' ';
  
  /* Print the formatted message into the buffer: */
  va_list ap;
  va_start(ap,whatFormat);
  size_t size=(whatEnd-1)-whatPtr; // Leave room for the error code suffix
  size_t length=vsnprintf(whatPtr,size,whatFormat,ap);
  
  /* Advance the buffer pointer by how much vsnprintf actually printed: */
  if(length<=size)
    whatPtr+=length;
  else
    whatPtr+=size;
  va_end(ap);
  
  /* Print a description of the error code into another buffer: */
  char description[1024]; // Max size according to specification
  strerror_r(errno,description,sizeof(description));
  
  /* Print the error code suffix into the buffer: */
  snprintf(whatPtr,whatEnd-whatPtr," due to error %d (%s)",errno,description);
  
  /* Create and return an exception object: */
  return std::runtime_error(whatBuffer);
  }

I don’t like that I am using a second buffer to call strerror_r, but having strerror_r write directly into the message buffer would require replacing the snprintf call with a lot more custom code, for which I am just too lazy. Again, this is only called in case of severe errors that prevent completion of whatever the current task is. Let’s not worry about it too much.

Let’s also not worry about what happens to the error message if they do get too long for the buffer. The way it’s coded, there will never be a buffer overrun, but long function names, long formatted messages, and long error code suffixes will get truncated in strange ways. It’s fine.

Anyway, with all that, we can now throw C library exceptions like this:

throw Misc::libcErr(__PRETTY_FUNCTION__,errno,"Cannot open file %s",fileName.c_str());

… and that looks pretty good to me.

Future Work

With this code in place, I can see some further improvements. Maybe C library errors should be thrown as a separate class derived from std::runtime_error that includes the original errno value as an integer to enable error handlers to do some targeted clean-up or try to actually fix the problem. Similarly, make all errors thrown by Misc::stdErr a new class derived from std::runtime_error that has methods to extract the error location and message separately. But not right now.

And Now, The Punchline

Why am I writing this post? This is pedestrian stuff, and not very elegant. But while thinking about improving error handling in Vrui, I ran into a curious issue. As I said above, using printf to assemble error messages seems, I don’t know, quaint. Un-C++-like. So I thought: why not use C++ streams to do it? The constraint, as before, is that throwing an error message has to fit into a single line, and by that I mean single unit of code.

We could use C++ string streams for that. Write a message with parameters into a string stream using insertion operators as usual, then retrieve the stream’s C++ string, put it into a new std::runtime_error object, and finally throw that. We don’t want to throw the string stream itself, of course, because it’s not in the exception class hierarchy and wouldn’t be caught by existing code. Using this in a method would look somewhat like this:

throw std::runtime_error((std::stringstream()<<Misc::parsePrettyFunction(__PRETTY_FUNCTION__)<<": Unable to open file "<<fileName<<" due to error "<<errno<<" ("<<strerror(errno)<<")").str());

… and wow, that’s ugly as sin. More parentheses than Lisp. And, yes, we would have to make a thread-safe wrapper for strerror that returns a C++ string, but that’s simple enough.

We can compress this code from the middle, and fix the strerror issue while we’re at it. First, instead of creating an un-named std::stringstream object and streaming the parsed __PRETTY_FUNCTION__ result into it, we create a wrapper class that does it for us:

class StartError:public std::stringstream
  {
  /* Constructors and destructors: */
  public:
  StartError(const char* prettyFunction)
    {
    (*this)<<Misc::parsePrettyFunction(prettyFunction)<<": ";
    }
  };

But then we still need to append the error code suffix, take the stream’s string, wrap it in a std::runtime_error, and throw it. And here is where I went a little mad: What if we had a class that, when inserted into a std::stringstream via operator<<, took the current contents of that stream, wrapped a std::runtime_error around it, and instead of returning the stream object, returned that error object, in order to be thrown? While at it, we could even insert the error code suffix automatically. Well, easy enough:

class EndLibcError
  {
  /* Elements: */
  private:
  int errorCode;
  
  /* Constructors and destructors: */
  public:
  EndLibcError(int sErrorCode)
    :errorCode(sErrorCode)
    {
    }
  
  friend std::runtime_error operator<<(std::ostream& os,const EndLibcError& error)
    {
    /* Append the C library error suffix to the stream: */
    os<<" due to error "<<error.errorCode<<" (";
    char description[1024];
    os<<strerror_r(error.errorCode,description,sizeof(description));
    os<<")";
    
    /* Check if the stream is a std::stringstream: */
    std::stringstream* sos=dynamic_cast<std::stringstream*>(&os);
    if(sos!=0)
      return std::runtime_error(sos->str());
    else
      return std::runtime_error("Whoa there!"); // Not gonna happen!
    }
  };

Now we can shorten the initial code to this:

throw StartError(__PRETTY_FUNCTION__)<<"Unable to open file "<<fileName<<EndLibcError(errno);

… which actually looks decent! Of course, there would be a simpler EndError class for non-C library errors that doesn’t append the error code suffix, as in

throw StartError(__PRETTY_FUNCTION__)<<"Something went terribly wrong!"<<EndError();

… But Here’s The Kicker (Insert Drum Roll)…

There is a bug in g++! The above code does not work! Apparently, when our custom operator<< is applied to the un-named StartError object, g++ ignores the fact that our operator returns a std::runtime_error object, and continues working under the assumption that our operator returned the StartError object that was its first parameter. Meaning, the code will throw a StartError object, not a std::runtime_error one! g++ does call our custom operator<<, and our operator does create the std::runtime_error object and returns it, but the calling code then immediately discards it and even calls its destructor, as if that’s what it thought it was supposed to do.

Why do I think it’s a bug in g++ and not my abominable code’s fault? First, overloading operator<< with a different return type, while certainly odd in the context of stream insertion, is definitely neither illegal nor undefined behavior. After all, it was the stream insertion mechanism that changed the return type of the original operator<< in the first place, which, as in int foo=bar<<2, does neither change nor return its first parameter.

Second, the above code does work exactly as intended if we use a named StartError object instead of an un-named one:

StartError err(__PRETTY_FUNCTION__);
throw err<<"Unable to open file "<<fileName<<EndLibcError(errno);

But, obviously, having to create a named object breaks our goal of doing this in a single code unit, so it’s not going to work.

It’s not an optimization setting or compiler flag issue, either. I tried all kinds of different settings, and they all exhibited the same behavior: the return value of our operator<< is deconstructed — properly! — and discarded, if and only if the first parameter to operator<< is an un-named stream object. That looks pretty cut and dry to me. But of course, dear reader, feel free to leave a comment and tell me how I’m wrong. 🙂

Off to rediscover how to file a gcc bug report…