Unnecessary const referencing

[J.Hilk]

@JonB This is allowed because const is purely an implementation detail when passing by value

[Chris Kawa]

There's nothing "technically" wrong here. The problem here is bad API design. On one hand it tries to be generic, so it doesn't restrict the type and uses const& for cases where you pass some "big" type that benefits from passing by reference. On the other it pessimizes the actually intended use case, which are the basic numeric types. So basically it's the worst of both worlds. It's optimized for case it won't be used for.
A good API design would first analyze use cases. If it's just for the basic types you can restrict the allowed types with something like std::is_arithmetic or a concept and pass by value. If it's for any type, but you don't want to loose on performance for any case then provide an overload for the two ways.

[Christian Ehrlicher]

What's wrong with

template <typename T>
bool Numbers::isPerfectPower(T &&number, int n);

:)

https://godbolt.org/z/afW38b9e8

[Chris Kawa]

@Christian-Ehrlicher said:

What's wrong with

This:
https://godbolt.org/z/bxcs7b355

[JonB]

God I love this --- an assembly language debate! :D

[J.Hilk]

@JonB said in Unnecessary const referencing:

God I love this --- an assembly language debate! :D

[kshegunov]

@Chris-Kawa said in Unnecessary const referencing:

@Christian-Ehrlicher said:

What's wrong with

This:
https://godbolt.org/z/bxcs7b355

Of course one would do something like this, instead:

template <typename T>
std::remove_reference_t<T> foo1(T&& a) { return std::forward<T>(a) + 42; }

https://godbolt.org/z/8nT5verW4

Granted, it still misses the opportunity to use lea instead of resolving the address through mov but works with whatever you pass it. I'd argue that for numeric/primitive types the compiler should expand a const lvalue to a copy at the optimizer pass, but it apparently doesn't.

[JonB]

@kshegunov
Why write this getting-more-complex-all-the-time code if you can instead write:

template <typename T>
bool Numbers::isPerfectPower(T number, int n);

(given usage for POD numerics)? I just don't get how having to know about and add in

std::remove_reference_t<T>
std::forward<T>

is anything other than making C++ harder. I like C++, but what you're asking me, the programmer, to remember is getting out of hand.....

[Chris Kawa]

@JonB said:

Why write this getting-more-complex-all-the-time code if you can instead write

Because nothing in that syntax prevents you from passing e.g. a BigNum class instance, that would be heavily penalized for using a by value parameter (copy).

That's why I mentioned restricting possible inputs e.g.

template<typename T>
concept ByValueType = std::is_arithmetic_v<T>;

template <ByValueType T>
bool Numbers::isPerfectPower(T number, int n);

or whatever other condition you think is appropriate, like checking the sizeof of the type to see if it fits in a register or using some custom type traits to separate types that should be passed by value from those that benefit from referencing.

Of course all of this discussion is academic given how wasteful the actual implementation of the OPs function is. Any benefits we come up on the parameter front are immediately buried just few lines into the function body, but hey, why not :)

[Chris Kawa]

@kshegunov Sure, but that's kinda another topic. What you show is just the intended benefit of small perfectly forwarded functions i.e. they completely disappear via inlining. That wouldn't happen though in a longer function, like what the OP has, so the benefit you show is indeed valid, but not really related to the topic at hand. Notice that you're actually doing what I proposed i.e. have an overload for copy and ref variants. The inlined && forward is just an unrelated complication for style points.

Btw. I love how the float version transforms into a fixed point xmm arithmetic with magic constant. Compilers are so smart these days :)

[kshegunov]

@Chris-Kawa said in Unnecessary const referencing:

Notice that you're actually doing what I proposed i.e. have an overload for copy and ref variants.

No argument there.

The inlined && forward is just an unrelated complication for style points.

As far as we are talking + 42, if for example the body was something like:

template <typename T>
std::remove_reference_t<T> foo1(T&& a) {
    return smthng(std::forward<T>(a)) + 42;
}

Where smthng is some other templated nonsense (or simply overloaded), then, I'm sure you'd agree the forward is no longer redundant. :)

Btw. I love how the float version transforms into a fixed point xmm arithmetic with magic constant. Compilers are so smart these days :)

Yeah, true enough. Although you can still catch them missing optimization opportunities from time to time.

@JonB said in Unnecessary const referencing:

Why write this getting-more-complex-all-the-time code if you can ...

What Chris said (... and since this is indeed an academic discussion at this point).