The perils of implicit conversions in C++

In my last post, I was boasting about improved simulator performance in Ymer. Unfortunately, the results for the tandem queuing network were invalid due to a bug introduced in Ymer 3.0.6. I fixed the bug and quickly released version 3.0.7. As seen below, there is still a boost from getting rid of the Rational class in Ymer—just not quite as big of a boost as I first had thought.

The bug was due to implicit conversion from const char* to bool. The Rational class had a single-argument constructor that parsed a C string into a rational value. The replacement TypedValue class did not have a corresponding constructor, but it did have a constructor for implicitly converting values of type bool to TypedValue. As it turns out, C++ permits implicit conversions from any pointer type to bool, so inadvertently there still was a way to implicitly convert a C string to a TypedValue. This implicit conversion was happening in the code that parsed constant overrides (needed for the tandem queuing network model), which resulted in all parsed constants to have value 1 (non-null pointer converted to bool). It explains the artificially flat performance across tandem queuing network model sizes for Ymer 3.0.6, and in hindsight I should have been more suspicious about it.

To prevent future occurrences of the same bug, I have added a private constructor to the TypedValue class that matches any pointer type. When the compiler encounters an attempt to implicitly convert a pointer type to TypedValue, an error is emitted because the matching constructor is private. For further protection, the constructor does not have an implementation, so we would still get a link error if we invoke the constructor in a context where it is accessible.

class TypedValue {
 public:
  enum Type { INT, DOUBLE, BOOL };

  TypedValue(int i) : type_(INT) { value_.i = i; }
  TypedValue(double d) : type_(DOUBLE) { value_.d = d; }
  TypedValue(bool b) : type_(BOOL) { value_.b = b; }

  Type type() const { return type_; }

  template <typename T> T value() const;

 private:
  template <typename T>
  TypedValue(T*);

  Type type_;
  union {
    int i;
    double d;
    bool b;
  } value_;
};

This bug, of course, demonstrates the importance of tests. There should have been a test that exposed the bug before it got released. I had actually added a unit test for the TypedValue class using the Google C++ testing framework, but needless to say it did not cover the erroneous conversion from a C string to TypedValue that happened in the parsing of constant overrides provided as a command-line argument.