CODEWITHSUNDEEP
c++
Lightness Races in Orbit
Lightness Races in Orbit

Reputation: 385144

As far as I can tell, none of these answers are correct. What am I missing?

In compiling some interview test questions, I'm currently taking examples from various sources and running through them to gauge their difficulty level and correctness. I've come across one that I think is broken, but it's also possible I'm missing something: if I am, I want to know, not only for my own knowledge but then it would also indicate that this is potentially a good, tricky question. I'd like you to help me regain my sanity and re-affirm the trust I have placed in myself. :D

What is the correct way to cast p at the placeholder "???" in the following code?

#include <iostream>

using namespace std;

uint16_t hash(void *p)
{
    uint32_t val = ???;
    return (uint16_t)(val ^ (val >> 16));
}

int main(int argc, char *argv[])
{
    uint32_t a[20];

    for(uint32_t i = 0; i < 20; ++i)
    {
        a[i] = i;
        cout << hash(a + i) << endl;
    }
}

Choose one:

  • static_cast<uint32_t>(p)
  • dynamic_cast<uint32_t>(p)
  • reinterpret_cast<uint32_t>(p)
  • const_cast<uint32_t>(p)

Ignoring for a moment that the call to hash must be ::hash to guarantee disambiguity with the standard library (e.g. this line doesn't compile for me in GCC 5.3.0, C++14 mode), I have a problem with the question.

Firstly, it's not clear what the program's supposed to do. Hash the array values, or hash the element locations? Because at the moment the function is receiving pointers-to-elements, but all the available answers assume that those pointers are to be themselves converted to uint32_t and used as the hash value. If that's the case, then even if you use a reinterpret_cast then there is a bug because sizeof(void*) may not be sizeof(uint32_t); val in that function should be intptr_t instead. The use of uint32_t for the type of the array elements themselves just confuses matters further if that's effectively a co-incidence.

Or the function is supposed to hash the value and the correct answer is not in the list: *static_cast<uint32_t*>(p).

The "correct" answer is apparently reinterpret_cast<uint32_t>(p), which makes me think the intention of the program is to hash the array element addresses.

Am I imagining these problems?
Is the question clear and is the solution one of the four choices offered?

Upvotes: 6

Views: 476

Answers (2)

Barry
Barry

Reputation: 302862

Just to summarize all the points you made in the question and the comments. Basically, the answer to this question is probably:

The correct cast to use is reinterpret_cast1†‡.

That's a lot of footnotes! I guess that's what makes it a real C++ question. In any event, I wouldn't ask it in an interview – it's too trick question – and there's plenty of real questions with real, direct answers that you can ask instead.

1 On a 64-bit system, you can't any_kind_of_cast a void* to a uint32_t. So they're all wrong.

† There's a name lookup issue in C++11 between the user-provided hash() and the the class template std::hash<T>. You should obviously link them to why is using namespace std; considered bad practice?

‡ Even if the above weren't problems that prevented the code from compiling in the first place, is the intent here to actually hash the pointers into a and not the values of a? The most-likely-correct answer should be:

auto val = *static_cast<const uint32_t*>(p);

If it is about hashing pointers, then reinterpret_cast would be the correct cast but not to uint32_t (as per (1)). We would want to use uintptr_t:

auto val = reinterpret_cast<uintptr_t>(p);

In the case of 64-bit, you'd probably want to do something either than val ^ (val >> 16), since you're effectively ignoring the upper 32-bits, but at least this would get you to a correct cast that would compile everywhere.

Upvotes: 5

Sergio
Sergio

Reputation: 1299

  1. IMHO you are imagining these problems :) The example was taken from here : reinterpret_cast So it is better to look at example in context of that article.

  2. Question is clear but when I finished reading it and all comments I've started doubt in my 1st minute solution to use reinterpret_cast (I would use it anyway as an answer even without context)

Upvotes: 4

Related Questions