0

I've been playing with C++ recently, and I just stumbled upon an interesting precedence issue. I have one class with two operators: "cast to double" and "+". Like so:

class Weight {
  double value_;
 public:
  explicit Weight(double value) : value_(value) {}
  operator double() const { return value_; }
  Weight operator+(const Weight& other) { return Weight(value_ + other.value_); }
};

When I try to add two instances of this class...

class Weighted {
  Weight weight_;
 public:
  Weighted(const Weight& weight) : weight_(weight) {}
  virtual Weighted twice() const {
    Weight w = weight_ + weight_;
    return Weighted(w);
  }
};

...something unexpected happens: the compiler sees the "+" sign and casts the two weight_s to double. It then spits out a compilation error, because it can't implicitly cast the resulting double back to a Weight object, due to my explicit one-argument constructor.

The question: how can I tell the compiler to use my own Weight::operator+ to add the two objects, and to ignore the cast operator for this expression? Preferably without calling weight_.operator+(weight_), which defeats the purpose.


Update: Many thanks to chris for pointing out that the compiler is right not to use my class's operator+ because that operator is not const and the objects that are being +ed are const.

I now know of three ways to fix the above in VS2012. Do see the accepted answer from chris for additional information.

  1. Add the explicit qualifier to Weight::operator double(). This doesn't work in VS 2012 (no support), but it stands to reason that it's a good solution for compilers that do accept this approach (from the accepted answer).
  2. Remove the virtual qualifier from method Weighted::twice, but don't ask me why this works in VS.
  3. Add the const qualifier to method Weight::operator+ (from the accepted answer).
Mihai Danila
  • 2,229
  • 1
  • 23
  • 28
  • You could make your conversion operator explicit. – chris Dec 01 '13 at 02:59
  • OK, a good idea, and it's probably OK to explicitly cast to `double` when I need one anyway. Why don't you make this into an answer? Nonetheless, I am having some troubles marking the operator explicit (the compiler now cries "illegal storage class"). Let me look into this a little closer. – Mihai Danila Dec 01 '13 at 03:00
  • @MihaiDanila: `explicit` for conversion operators was added in C++11. Depending on the compiler/version you're using, it may not be supported at all, or you may have to give a flag to tell it to support C++11. – Jerry Coffin Dec 01 '13 at 03:07
  • You're right. With VS 2012 express, it's not supported. Downloading 2013... Source: http://stackoverflow.com/questions/11365129/explicit-operator-bool-error – Mihai Danila Dec 01 '13 at 03:09
  • @chris: nonetheless, I thought the + operator would have a higher precedence than a cast operator, especially when one is not necessarily needed? So in line with that intuition, shouldn't the compiler prefer the specialized operator+ even without my massaging the cast operator? Especially since I have two `Foo`s as inputs and the result is assigned to another `Foo`. – Mihai Danila Dec 01 '13 at 03:12
  • @MihaiDanila, Turns out I missed the main point. let me update my answer. – chris Dec 01 '13 at 03:15
  • first make your operator double and + both `const`. Second, is not your `operator+` as written illegal? It returns a `double` when the `double` to `Foo` conversion is explicit. – Yakk - Adam Nevraumont Dec 01 '13 at 03:15
  • `operator double()` is already `const` in my IDE, thank you for the hint. The other one was not. Making it `const` seems to solve my original issue. How is that possible? – Mihai Danila Dec 01 '13 at 03:17
  • I've updated the snippet. Now it's exactly like in my IDE, minus the name of the class. This is the code that exhibits the issue. It seems to be due to the `operator+` not being marked `const`. – Mihai Danila Dec 01 '13 at 03:20

1 Answers1

2

Current version:

First of all, the virtual should have nothing to do with it. I'll bet that's a problem with MSVC, especially considering there's no difference on Clang. Anyway, your twice function is marked const. This means that members will be const Weight instead of Weight. This is a problem for operator+ because it only accepts a non-const this. Therefore, the only way the compiler can go is to convert them to double and add those.

Another problem is that adding explicit causes it to compile. In fact, this should remove the compiler's last resort of converting to double. This is indeed what happens on Clang:

error: invalid operands to binary expression ('const Weight' and 'const Weight')
Weight w = weight_ + weight_;
note: candidate function not viable: 'this' argument has type 'const Weight', but method is not marked const

Finally, making operator+ const (or a free function) is the correct solution. When you do this, you might think you'll add back this route and thus have another error from ambiguity between this and the double route, but Weight to const Weight & is a standard conversion, whereas Weight to double is a user-defined conversion, so the standard one is used and everything works.


As of the updated code in the question, this is fine. The reason it won't compile is the fault of MSVC. For reference, it does compile on Clang. It also compiles on MSVC12 and the 2013 CTP.


You may be storing the result in a Foo, but there is still an implicit conversion from double to Foo needed. You should return Foo(value_ + other.value_) in your addition operator so that the conversion is explicit. I recommend making the operator a free function as well because free operators are (almost) always at least as good as members. While I'm at it, a constructor initializer list would be a welcome change, too.

In addition, from C++11 onward, a generally preferred choice is to make your conversion operator explicit:

explicit operator double() const {return value_;}

Also note the const added in because no state is being changed.

chris
  • 60,560
  • 13
  • 143
  • 205
  • Nice, and `operator+` can also be made `const`. – Mihai Danila Dec 01 '13 at 03:15
  • Chris, you are right about the return, but it seems the solution is to make `operator+` `const`. That alone removes my compilation error, without making the cast operator `explicit`. – Mihai Danila Dec 01 '13 at 03:18
  • @MihaiDanila, Not in Clang it doesn't. I'm willing to bet it's another small problem with MSVC. – chris Dec 01 '13 at 03:19
  • Understood. I can't verify the `explicit` piece, but it sounds reasonable. So I will mark your answer as accepted answer. I would appreciate it if you put a footnote in it about how I solved it in MSVC, so that others who use MSVC 12 might benefit from it. – Mihai Danila Dec 01 '13 at 03:22
  • @MihaiDanila, I was about to :) – chris Dec 01 '13 at 03:23
  • Also, the error you mention in the answer, while it was a real error in my snippet, is not the source of the compilation error. It's not even in my snippet anymore, so you might want to adjust the answer accordingly. – Mihai Danila Dec 01 '13 at 03:23
  • Thanks for the updates. If I may add, my error goes away in MSVC 12 if I just add `const` to `operator+`. – Mihai Danila Dec 01 '13 at 03:28
  • @MihaiDanila, I don't see how you could get an error in the first place with MSVC12 (which is the compiler in VS2013). I had a completely successful build. – chris Dec 01 '13 at 03:37
  • let us [continue this discussion in chat](http://chat.stackoverflow.com/rooms/42281/discussion-between-mihai-danila-and-chris) – Mihai Danila Dec 01 '13 at 17:50
  • Excellent explanation; this clarifies why `const` on `operator+` fixes the issue. It doesn't explain why removing `virtual` also fixes the issue, but makes a bug more understandable if MSVC somehow gets confused about the qualifiers in the absence of `virtual`. I just wish the compiler were smart enough to tell me about this ("couldn't use the more specific operator+ because of this and that"). – Mihai Danila Dec 01 '13 at 19:10
  • one more question after rereading your answer: I thought that marking a method like `twice` const only implied that I couldn't write to its instance. From your answer, it seems that the `const` method treats all the instance members as `const` all throughout. Probably to protect the state of the members themselves from accidental writing, which makes perfect sense now. Is this why `weight_` is treated as `const`? – Mihai Danila Jan 08 '14 at 05:20
  • @MihaiDanila, That's really better as a separate question or research on `const`. – chris Jan 08 '14 at 13:01