3

I got a wrapper class that has a simple and light-weighted implicit conversion operator to double.

I like to use it as I would use a double, for example:

if (!std::isfinite(myVar)) ...

But visual c++ implementation of std::isfinite(double) is actually a template that get his argument by copy.

So my wrapper class copy constructor is called, and it is not light-weighted.

To avoid this, I have to write:

if (!std::isfinite((double)myVar)) ...

for every call :(

If visual c++ std::isfinite() was defined as is it on cppreference.com, I would not have to cast every call: ([edit] I may be wrong, Integral isn't an actual type... but... [edit] It still should not accept user defined types ?)

bool isfinite( float arg );
bool isfinite( double arg );
bool isfinite( long double arg );
bool isfinite( Integral arg );

I am not sure what the standard says about this. Is vc++ template std::isfinite standard-conforming ?

Should I report this as a bug on Microsoft connect ?

Should I define my own isfinite(double) that calls std::isfinite ?

edit

Or maybe it is a non-issue as in a release build the calls get inlined and no copy occurs ? (well I'll try to check it right now and update in a few minutes)

edit 2

It doesn't seem to get inlined in a release build with /Ob2 (inline any suitable function)

edit 3

as requested, a sample:

struct DoubleWrapper {

  double value;

  DoubleWrapper(double value) : value(value) {
    printf("copy Ctor\n");
  }

  DoubleWrapper(const DoubleWrapper & that) : value(that.value) {}

  operator double() const {
    return this->value;
  }
};

int main() {

  DoubleWrapper a(rand());      //rand to prevent optimization

  auto res = std::isfinite(a);

  printf("%d", res);            //printf to prevent optimization
}

edit 4

So, based on Ben Voigt comment, this is what I added to my class header:

#include <cmath>

namespace std {
    inline bool isfinite(const DoubleWrapper<double> & dw) {
        return isfinite((double)dw);
    }
}

Is this a correct solution ?

The only thing is, should I do the same for all <cmath> functions that takes a double ?

edit 5

I'm responding to Shafik Yaghmour answer here, because a comment is too limited (maybe I should start a new question)

If I understand correctly, instead of my edit 4, I should add this to my class header:

inline bool isfinite(const DoubleWrapper<double> & dw) {
    return isfinite((double)dw);
}

using std::isfinite;

Is it required to put it in a namespace, or can I leave it in the "global" namespace ?

But this mean I have to change all my calls from std::isfinite(dw) to isfinite(dw).

Ok, but I realize there are many things I don't understand. I am confused.

I understand that adding an overload to std is not allowed.

However, adding a template specialization is allowed ? Why ? What difference does it makes ?

Anyway, I tried it, and it is not a solution to my problem because this specialization:

template<> inline __nothrow bool std::isfinite(const MagicTarget<double> & mt) {
    return std::isfinite((double)mt);
}

will not be selected by the compiler over the standard one:

template<class _Ty> inline __nothrow bool isfinite(_Ty _X)
{
    return (fpclassify(_X) <= 0);
}

To be selected, it should be 

template<> inline __nothrow bool std::isfinite(MagicTarget<double> mt) {
    return std::isfinite((double)mt);
}

But this would still call the copy Ctor :(

Surprisingly the overload (see edit 4) is selected over the standard template... I am beginning to think that some C++ rules are too subtle for me :(

But, to begin with, why on earth are cmath functions and especially std::isfinite a template ?

What's the point of accepting anything else that floating points types ?

Anyway, vc++ std::isfinite calls std::fpclassify that is only defined for float, double and long double. So... What's the point?

I am thinking the standard committee screwed up by allowing cmath functions to be templates. They should only be defined for the types that are relevant, or maybe takes their arguments as universal references.

That's it, sorry for the rant...

I will go for the (not in std) overload. Thank you!

ThreeStarProgrammer57
  • 2,906
  • 2
  • 16
  • 24
  • 1
    `Integral` isn't an actual type, and the reason you're seeing a template. – MSalters Jun 02 '15 at 12:19
  • 1
    http://stackoverflow.com/questions/14580168/stdisfinite-on-msvc this might help. – Tejendra Jun 02 '15 at 12:19
  • 3
    _"implicit conversion operator to double"_ Don't do that. Stick with the casts, or wrap `isfinite`. – Lightness Races in Orbit Jun 02 '15 at 12:21
  • 1
    _"Or maybe it is a non-issue as in a release build the calls get inlined and no copy occurs ?"_ Copy elision (what you're calling "inlining") still requires that the copy be at least possible. That's why `std::mutex m = std::mutex()` is ill-formed, even though in practice that is never going to actually involve copy-initialisation. – Lightness Races in Orbit Jun 02 '15 at 12:22
  • 1
    @LightnessRacesinOrbit So what is the better approach ? remove implicit conversion and always call a conversion member function like `myVar.ToDouble()` ? – ThreeStarProgrammer57 Jun 02 '15 at 12:24
  • 1
    @RealProgrammer57: Yeah or the `(double)` you were already doing (`explicit operator double() const`) – Lightness Races in Orbit Jun 02 '15 at 12:25
  • @LightnessRacesinOrbit By "wrap `isfinite`" you mean a `isfinite()` member function for my wrapper class ? – ThreeStarProgrammer57 Jun 02 '15 at 13:04
  • 7
    @RealProgrammer57: Write your own function in one of your own namespaces: `bool isfinite(const DoubleWrapper& dw) { return std::isfinite(double(dw)); }` – quamrana Jun 02 '15 at 13:08
  • @RealProgrammer57: Call it whatever you like – Lightness Races in Orbit Jun 02 '15 at 13:27
  • okay, thanks everyone! A last one: I would like to be able to write `myVar + 123.456`, rather than `double(myVar) + 123.456`. Is it a bad idea ? – ThreeStarProgrammer57 Jun 02 '15 at 13:35
  • @LightnessRacesinOrbit, (In case you didn't get notified) please see my previous comment (anyway, thanks again !) – ThreeStarProgrammer57 Jun 02 '15 at 14:03
  • 3
    @RealProgrammer57: That doesn't look so bad. It might make sense for your type to be able to directly engage in arithmetic. Just do it directly with an `operator+`, rather than going through implicit conversions to `double` first. – Lightness Races in Orbit Jun 02 '15 at 14:07
  • Why are you wrapping double? Can't you implement a member function on our wrapper that does this? – rubenvb Jun 02 '15 at 14:14
  • @rubenvb This is just a sample. My wrapper is in fact a "magic target" meant to be pointed to by one or more "magic pointers". "magic" means that both can be moved and the pointer will update itself to point to the new address of the "magic" target. The question was motivated by a `MagicTarget` that I want to use as if it was a double. Maybe "proxy" class would be a better name that "wrapper" class. – ThreeStarProgrammer57 Jun 02 '15 at 15:31
  • 1
    What quamra said. In addition,you can provide `std::isfinite` itself: "A program may add a template specialization for any standard library template to namespace `std` only if the declaration depends on a user-defined type and the specialization meets the standard library requirements for the original template and is not explicitly prohibited." – Ben Voigt Jun 02 '15 at 15:39
  • 1
    @HowardHinnant wait, why wouldn't conversions apply here? Also, the OP is assuming the copy is happening on the call to isfinite if I understand correctly, which is not the case. – Shafik Yaghmour Jun 02 '15 at 15:56
  • 1
    @HowardHinnant see Richard's [comment here](https://llvm.org/bugs/show_bug.cgi?id=18218#c8) this is also how I read it, which was why I was puzzled that you thought the current deleted answer was correct. – Shafik Yaghmour Jun 03 '15 at 04:20
  • @ShafikYaghmour In your link, does Richard's comment implies that vc++ `std::isfinite()` is indeed not standard-conforming ? – ThreeStarProgrammer57 Jun 03 '15 at 09:19
  • @ThreeStarProgrammer57 as far as I can tell it is under the current language but it seems that the intent is that isfinite and other math functions should only really apply to arithmetic types which a class with a conversion to double is not. Since according to [this defect report](http://cplusplus.github.io/LWG/lwg-defects.html#2086) Howard came up with the new wording I want to know what he says. – Shafik Yaghmour Jun 03 '15 at 09:22
  • @ShafikYaghmour: Richard makes a good point in that comment. I agree, he looks right. Depending on that subtlety may get you into trouble. If it were my code I would explicitly cast to the arithmetic argument when calling a `` function. – Howard Hinnant Jun 03 '15 at 14:32
  • @HowardHinnant completely agree, I would not rely on it. The desire to make this ill-formed like `std::abs(0u)` make sense to me, although I wonder how much code out there relies on this. – Shafik Yaghmour Jun 03 '15 at 14:37

1 Answers1

3

Addressing edit 4 to your question since through the comments you have come close to a final solution.

You should not be adding it to the std namespace, you should add it to one of your own namespace and rely on argument dependent lookup see Is it a good practice to overload math functions in namespace std in c++ for more details.

Given the standard committee's apparent desire to restrict cmath functions to be called with arithmetic types only, relying on an implicit conversion is not a good idea. So performing an explicit conversion via a cast is the safe way to go for all cmath functions:

isfinite((double)dw)

You can find the details of this in Is it valid to pass non-arithmetic types as arguments to cmath functions?.

Community
  • 1
  • 1
Shafik Yaghmour
  • 154,301
  • 39
  • 440
  • 740