10

I have an enum parameter in a C++ program that I need to obtain using a function that returns the value through a parameter. I started by declaring it as an int but at code review was asked to type it as the enum (ControlSource). I did this but it breaks the Get() function - I noticed that a C-style cast to int& resolves the problem, but when I first tried to fix it with a static_cast<> it didn't compile.

Why is this, and why is it that when eTimeSource was an int no casting is required at all to pass the integer by reference?

//GetCuePropertyValue signature is (int cueId, int propertyId, int& value);

ControlSource eTimeSource = ControlSource::NoSource;

pPlayback->GetCuePropertyValue(programmerIds.cueId, DEF_PLAYBACKCUEPROPERTY_DELAY_SOURCE, static_cast<int&>(eTimeSource)); //This doesn't work.

pPlayback->GetCuePropertyValue(programmerIds.cueId, DEF_PLAYBACKCUEPROPERTY_DELAY_SOURCE, (int&)(eTimeSource)); //This does work.

int nTimeSource = 0;
pPlayback->GetCuePropertyValue(blah, blah, nTimeSource); //Works, but no (int&) needed... why?
Gareth
  • 439
  • 5
  • 12
  • "Works, but no (int&) needed... why?" - Take a look in the C++ documentation regarding passing variables by reference (or see here: http://stackoverflow.com/a/410857/1174378 ) – Mihai Todor Mar 08 '13 at 17:18
  • Removed the double brackets. Am I right in presuming that the C-style cast is calling a reinterpret_cast on this object to make it work? If so, does it really matter for an enum that's based on the int type anyway? – Gareth Mar 08 '13 at 18:10
  • No, `reinterpret_cast` is not the same as a C-style "sledgehammer" cast either. If you need an ugly hacky cast to make the code work after changing the type then don't change the type! It works OK with `int`, it doesn't work OK with an enum ... seems pretty clear what the answer is to me – Jonathan Wakely Mar 08 '13 at 18:43

2 Answers2

8

When you convert a variable to a value of a different type, you obtain a temporary value, which cannot be bound to a non-constant reference: It makes no sense to modify the temporary.

If you just need to read the value, a constant reference should be fine:

static_cast<int const &>(eTimeSource)

But you might as well just create an actual value, rather than a reference:

static_cast<int>(eTimeSource)
Kerrek SB
  • 464,522
  • 92
  • 875
  • 1,084
  • In the cast of static_cast(eTimeSource) the compiler tells me that it cannot find an appropriate overload of the function and the same applies to static_cast(eTimeSource). I don't really want to update the GetProperty function to take a const reference as I suspect it would involve making a lot of changes elsewhere. – Gareth Mar 08 '13 at 18:06
  • 1
    @Gareth: Then make a local variable, `int i = static_cast(eTimeSource);` and call the function with `i`. – Kerrek SB Mar 08 '13 at 19:19
  • Fair enough - that would obviously work. It's a bit circular however as I started out by declaring my variable as an int in the first place, with no cast at all, but the reviewer didn't like it! I see his point - properly declaring the type makes it clear what it is for, but it brings this problem with it... – Gareth Mar 09 '13 at 00:21
  • @Gareth: Well, that's sort of the price you pay for the legacy code. The right thing would be to change the interface to accept the enum directly, but if you can't do that, you have to work around it. You could raise it with your reviewer. – Kerrek SB Mar 09 '13 at 12:26
3
static_cast<int&>((eTimeSource))); //This doesn't work.

Right, it doesn't work, because eTimeSource is not an int so you can't bind a int& to it. 

(int&)((eTimeSource))); //This does work.

Wrong, that doesn't work either, it just appears to. The C-style cast lies to the compiler and says "just make it this type, even if that's not legal". Just because something compiles doesn't mean it works.

why is it that when eTimeSource was an int no casting is required at all to pass the integer by reference?

Because you can bind an int& to an int but not to a different type, and eTimeSource is a different type. An int& is a reference to an int. If you could bind it to a different type it wouldn't refer to an int, would it?

If the code reviewer said to change the variable to an enumeration type they probably also meant for you to change the function parameter to take a ControlSource&

Jonathan Wakely
  • 166,810
  • 27
  • 341
  • 521
  • What if `ControlSource` is actually an `enum class`? Given the OPs code, that is an option AFAICS... – Daniel Frey Mar 08 '13 at 17:33
  • ControlSource is a C++ Native enum, not an enum class or ref enum. Additionally the reviewer did not want or expect the parameter to change to the enum type - the underlying data storage class relies on the value being typed as an int. It has been accepted that this should remain the case (as introducing the enum type would take too long - it is non-trivial as a GetMethod delegate signature further down the line can't tell the difference between the enum and the int and gets its overloads in a twist). Making the 'proper' signature work is not compatible with the time I have available ATM! – Gareth Mar 08 '13 at 17:49
  • @DanielFrey, would that change anything in my answer? (TBH I only re-read it briefly, so if it does please correct it) – Jonathan Wakely Mar 08 '13 at 17:53
  • Um, in this case it does *not* change anything, actually. Sorry, I was reading it too briefly :-/ – Daniel Frey Mar 08 '13 at 17:57
  • 1
    "Just because it compiles, doesn't mean it works". Out of curiosity, will this actually work anyway? Is there actually any risk in C-style casting between int and an int based enum at all? – Gareth Mar 09 '13 at 00:24
  • 3
    What do you mean by work? I think `(int&)(eTimeSource)` is equivalent to `const_cast(static_cast((int)eTimeSource))`, which creates a temporary integer, binds a const-reference to it, then casts away the const. If the function modifies the argument (which I assume it does, since it takes a non-const reference) then it will modify the temporary, it won't change `eTimeSource` ... so you lose the value that got set. I doubt that qualifies as working correctly. – Jonathan Wakely Mar 09 '13 at 00:43
  • @JonathanWakely I just tried the C-style cast and it does seem to work and write to the original variable, so I think your explanation with the temporary is incomplete. – nog642 Apr 14 '22 at 17:44