6 comments on “Overthinking: A critique of Herb Sutter’s recommendations on function arguments

  1. For a time, Scott Meyers proposed that the standard for setters be strictly rvalue references: void setValue(Thing&& value). I’m not sure if he still subscribes to this approach, but I think it is the best approach. Since the setter is going to keep the value either way, it forces the caller to be quite explicit about their intent. If the caller wants to submit a copy, it needs to make the copy:

    setValue(string(myOtherString));

    If the caller wants to gut an existing value, it needs to say so:

    setValue(move(myOtherString));

    Thoughts?

    • I hadn’t heard that idea before. Are you sure it’s not the one where he suggests using a forwarding reference (what he calls a universal reference)? I know he’s suggested doing something like:

      template <typename T>
      void set_name(T&& s)
      {
        static_assert(std::is_same<std::decay_t<T>,
          decltype(s_)>::value, "");
        // Or I suppose you could use is_assignable
        
        s_ = std::forward<T>(s);
      }

      But if you actually mean a rvalue reference and not a forwarding reference – so that lvalues can’t bind to it – I haven’t heard that before and I don’t think that’s his current advice. Judging by Effective Modern C++ – which is still fairly hot off the presses – he’s on board with the pass-by-value by default scheme.

      At a glance, I can’t say I see the point of disallowing lvalues. Generally speaking, the only time you want to make your type’s interface harder to use is when you’re trying to prevent users from doing something dangerous; you make the interface clunkier to force them to think about what they’re doing, and explicitly spell out what they want if they really, really mean it.

      Refusing lvalue arguments seems a little gratuitous. You’re not making the type any safer, just harder to use. That seems a little perverse for a general design guideline.

      I suppose it’s slightly faster in certain cases than pass-by-value, but I would argue you should make your type usable first, optimized second.

      I’d be curious to hear his arguments for why this is a good idea. If it’s just based on the same reasoning as Sutter’s second argument – where he’s concerned about the noexcept being “misleading” – my response would be the same as the one I gave above: don’t write shitty code to pander to clueless programmers, educate the programmers. If it’s to avoid code duplication and only write a single setter – rather than one for const lvalue references and one for rvalue references – without the complication of a forwarding-reference template like the one above, I’d say it’s a little nasty to punish the users of your type this way; suck it up and either write the forwarding template or the const-lval-ref/rval-ref pair – if you really need that extra little bit of efficiency so badly you’re considering penalizing users of the type for it, a duplicate function or a template is really a negligible price to pay compared to pissing off the users.

      • He has an old post on move-only types. It’s not quite the same as what I as describing, but it’s as close as I can get to what I’m proposing.

        http://scottmeyers.blogspot.com/2014/07/should-move-only-types-ever-be-passed.html

        Aside from that, yes, I was referring to explicit types, not universal references. The cost of two move constructions is unlikely to ever be a problem. I certainly prefer a function take the object by value as opposed to by const ref as Herb seems to recommend. So, in the long run, I wouldn’t complain working in a code base that consisted of setters taking resources by value.

        It’s not that I think programmers are stupid, but I feel that deep copying objects should be an operation you explicitly ask for. I understand why C++ is structured the way it is, but I’m not terribly fond of the end result: that something small like an int is treated more-or-less the same as something bulky like a vector or map (in that copying the value is normal operation). I like how newer languages (such as Rust) lean toward moving complex resources by default. It’s more philosophical than anything.

        • I can see his argument for only taking move-only types by rvalue reference in sinks. It seems illogical to force a copy for types that can’t be copied (because they’re move only).

          By exactly the same logic it seems illogical to force a move for non-moveable copy-only types – hence you should take copy-only types by value in sinks, not by rvalue. And by the same logic, if a type can be copied or moved, you should take it in a way that allows it to be copied or moved: which means either by value, by pair of overloads, or by forwarding reference.

          I get the philosophical position you’re coming from, but I think it’s misguided. You are too focused on expensive-to-copy types like strings and containers. The *VAST* majority of types in C++ are quite quick to copy; those things that aren’t are often dealt with via handles (like standard locales for example – exception_ptr is another example). And even expensive-to-copy things are often set up for quick copying whenever possible (for example, the small-string optimization). On top of that, if you think about it, the number of cases where you are forced to copy is vanishingly small – none of the standard algorithms force copies (obviously not counting those that are explicitly for copying) – and in those rare cases where you actually do need a copy, well, you need a copy, so there’s no sense in fighting it. That includes sink functions.

          That is all by design; that is the C++ philosophy. Types are only expensive to copy if they absolutely have to be, and copies are avoided as much as possible. Going out of your way to “fix” the problem of expensive copies is really fixing a problem that doesn’t really exist.

          Here’s another way to look at it. When I call some function foo(data), all I care about (with regards to the argument) is whether foo() will mutate data (which I can determine by how it takes data – either by-val/by-const-lval-ref, or by-non-const-lval-ref/by-rval-ref). If it’s not going to mutate data, it’s not really my concern as the caller whether foo() makes a copy of data or not. If it has to, it will; if it doesn’t, it won’t – that’s all internal logic. Why take it upon yourself to hoist the “do-I-need-to-copy” logic out of foo() and into the code that uses foo()? If you’re concerning yourself with what happens in the guts of every function you call – whether it copies the arguments internally or not – you’re kinda missing the point of functions. Just let the function do whatever it needs to do.

          • I know. I certainly won’t win this argument with the C++ community at large. I’m more exploring it from a language design angle. By and large, I agree with you that taking a value is a clean, semantic way to get what you want.

            I should clarify my position: setters should only take rvalue references for types that support it. I wouldn’t have a setValue(int&& value).

  2. Another way to phrase Herb Sutter’s point is to say that there is a real downside to coding move semantics in for a method. Your choices are:

    1) const & + &&
    2) by value
    3) perfect forwarding

    The disadvantages are :
    1) Code duplication (exponential in # of args).
    2) Worse performance for copying, suboptimal performance for moving, slicing, doesn’t do conditional copying.
    3) Code complexity (extreme).

    All these approaches have serious disadvantages.

    It is for this reason that your claim that any c++ programmer expects that set_name(move(s)) is faster than set_name(s) is such a whopper. This point is basically circular: if all the techniques including 2) did not have major downsides, then move semantics would be easy and downside-free to implement, and then such an expectation would be reasonable. But the fact that all three approaches, including 2), have major downsides, is Sutter’s whole point (restated), and why he recommends not implementing move semantics by default. If you agree with Sutter, then you wouldn’t have this expectation. So this doesn’t present anything against his case.

    Incidentally, I think regardless of my personal views it would be incredibly naive to assume that a setter in random third party library code would be faster on an rvalue than an lvalue. What’s more, if the function took its argument by const &, you would immediately know that the setter would not be faster, thus no expectations are broken.

    As to optimization, when you don’t deeply care about speed, it’s much better to optimize the worst case at the expense of the best, than vice versa (which you seem to think is better). Less variance in your code’s performance means that its easy to draw conclusions with less effort. You could use option 2) having not seen Sutter’s talk, test it, performance looks good, then a user comes along and shows you how repeated setting is really slow. You’d be shocked. Whereas const & will always have the same performance, so you are less likely to be surprised.

    As for throwing, I think it’s pretty clear why the situation that Sutter proposes is better: for the simple reason that you never promise nothrow when it isn’t (meaningfully, as opposed to technically) upheld. It’s better to under promise than over promise. When you use technique 2), you can write noexcept on a technicality, but still cause exceptions. That’s bad. You can only meaningfully promise noexcept on a function that moves, not on a function that copies. So if you want to “optimize” your promises (and given that each function has to make its own promise), you need to have two functions.

    I think that two things have to be distinguished: first, how you write a setter by default, and second, how you write a setter once you decide you want move semantics. Sutter’s point, which I think frankly is very hard to argue with, is that the default should be just const &, because you don’t need move semantics by default. In most cases it just doesn’t matter. You call Sutter’s approach premature optimization, but that’s exactly backwards. Worrying about move semantics in the first place is a premature optimization. Once you decide you need move semantics, you should decide based on your three options, but you shouldn’t accept any of the disadvantages these techniques bring if you don’t need move semantics in the first place.

Leave a Reply

Your email address will not be published. Required fields are marked *