0

I'm trying to use clamp, but can't find it

A summary example

File: test.h

#ifndef DEMO_H
#define DEMO_H

#include <algorithm>

namespace demo {

   typedef float real_t;
   class test {
       public:
           void start();
           real_t x = 10.90;
           real_t y = 2.10;
       test();
       ~test();
   };
}

#endif //DEMO_H

File: test.cpp

#include "test.h"

using namespace demo;

void test::start() {
    const int& p = std::clamp(x, 0,  y);
}

VSC error, compile using scons:

no overloaded function instance "std :: clamp" matches the argument list - argument types are: (real_t, int, real_t)

Thanks a lot!

jbelenus
  • 483
  • 1
  • 7
  • 18
  • 2
    Does your compiler support C++17? – UnholySheep Apr 10 '20 at 12:42
  • 2
    Requires c++17; can you show the command line used to compile please. – Richard Critten Apr 10 '20 at 12:42
  • 2
    `clamp` needs c++17. What version are you compiling with? – cigien Apr 10 '20 at 12:42
  • Which compiler you are using? Are you building this on Visual Studio? – dj1986 Apr 10 '20 at 12:44
  • Unrelated to error: `x` will be a dangling reference after the line `const int& x = std::clamp(2, 0, 255);` because `std::clamp` returns a reference to one of its parameters, all of which are referencing temporaries that will be destroyed at the end of the full-expression. And so using `x` after that line will cause undefined behavior. Change `const in&` to `int` to obtain the result by-value. – walnut Apr 10 '20 at 12:46
  • @walnut Won't this be fine? I thought the `const int&` would extend the lifetime of the temporary. And only if it were `int&` that would be wrong? – cigien Apr 10 '20 at 12:50
  • @cigien All references extend lifetimes, not only `const&`, but temporaries cannot be bound to non-const lvalue references. But the problem is that lifetime is extended only once, when the temporary is bound to a reference for the first time. This happens here in the function parameter, i.e. `2` is bound to the first parameter of `std::clamp` ("extending" its lifetime without any real effect), binding to the reference in the return value or `x` then does not further extend the lifetime. – walnut Apr 10 '20 at 12:53
  • @walnut Aah, I seem to have misunderstood something then. Thanks for the explanation. – cigien Apr 10 '20 at 12:57
  • The confusion comes from that, I need std::clamp use the real_t – jbelenus Apr 10 '20 at 15:10
  • @walnut I understand, do not notice this detail before (real_t) – jbelenus Apr 10 '20 at 15:14
  • The code is for Godot CPP: https://github.com/GodotNativeTools/godot-cpp/blob/master/include/core/Defs.hpp – jbelenus Apr 10 '20 at 19:43
  • 1
    I think the question can be voted to be reopened now. I have done so. However, you should still fix the title, which references a *different* error. – walnut Apr 10 '20 at 19:51
  • Ok, I was collaborative a little patience.. I'm trying to define my own clamp function THX! – jbelenus Apr 10 '20 at 19:58
  • 1
    The solution: https://github.com/GodotNativeTools/godot-cpp/blob/master/src/core/Color.cpp#L477 – jbelenus Apr 10 '20 at 20:10

1 Answers1

2

The problem is that std::clamp is declared as follows:

template<class T>
constexpr const T& clamp(const T&, const T&, const T&);

That means that all three arguments must have the same type. If the template argument is not explicitly specified, then deduction will fail, because successful template argument deduction requires that all function parameters from which a template argument is deduced yield the same type.

But in your example, you use:

std::clamp(x, 0,  y);

which uses function arguments of type float, int, float in that order. Therefore the first and third function argument will deduce T to float, but the second one will deduce it to int, causing deduction, and thereby overload resolution, to fail.

Make sure that you use the same types:

std::clamp(x, real_t(0), y);

In the line

const int& p = std::clamp(x, 0,  y);

you are using the wrong type for p. I assume that you meant it to be real_t, not int. You can avoid such type mismatches by using auto:

const auto& p = std::clamp(x, 0,  y);

Also, it is not save to store the result of std::clamp to a reference if not all arguments to std::clamp are lvalues.

std::clamp returns one of its arguments by-reference. So it may happen that the second argument real_t(0), which is a temporary, is returned by-reference. But that temporary is destroyed after the line

    const auto& p = std::clamp(x, real_t(0),  y);

so p might be a dangling reference. Instead store the result by-value:

    auto p = std::clamp(x, real_t(0),  y);

This technically doesn't apply to your line with variable type const int&, because the type mismatch between that and the returned reference from std::clamp will cause creation of a temporary which will then bind to p and be life-time extended. But this is not something one should rely on.


In a comment you link you should an alternative implementation for clamp as a macro.

A macro here as a significant drawback: It will evaluate its arguments multiple times. If you want a std::clamp alternative that accepts different types, then you can write it as a function template yourself, e.g.:

template<typename T, typename U, typename V>
constexpr auto clamp(const T& t, const U& u, const V& v) {
    return (t < u) ? u : (v < t) ? v : t;
}

Note that in this case it is important that the function returns by-value, because the ?: operator might result in a prvalue depending on the argument types.

Also think carefully about whether you actually want to use clamp with different argument types (This applies to both the implementation I show above as well as the macro you are using). You could easily accidentally make unintended comparisons, for example if one of the arguments is a signed integer and the other an unsigned one, the comparison with < will yield unintended results. This is probably why the standard library does not allow different argument types.

walnut
  • 21,629
  • 4
  • 23
  • 59
  • Thank you very much for the explanation and for the inconvenience, It is much cleaner, novice error. A hug! – jbelenus Apr 12 '20 at 14:25