0

This is my first experience with downcasting in C++ and I just can't understand the problem.

AInstruction and CInstruction inherit from AssemblerInstruction. Parser takes the info in its ctor and creates one of those derived instruction types for its mInstruction member (accessed by getInstruction). In the program, a method of the base AssemblerInstruction class is used, for happy polymorphism.

But when I want to test that the Parser has created the correct instruction, I need to query the derived instruction members, which means I need to downcast parser.getInstruction() to an AInstruction or CInstruction.

As far as I can tell this needs to be done using a bunch of pointers and references. This is how I can get the code to compile:

TEST(ParserA, parsesBuiltInConstants)
{
    AssemblerInstruction inst = Parser("@R3", 0).getInstruction();
    EXPECT_EQ(inst.getInstructionType(), AssemblerInstruction::InstructionType::A);
    AssemblerInstruction* i = &(inst);
    AInstruction* a = dynamic_cast<AInstruction*>(i);

    EXPECT_EQ(a->getLine(), "R3");
}

Running this gives this error: unknown file: error: SEH exception with code 0xc0000005 thrown in the test body.

And stepping through the code, when the debugger is on the final line of the function, a is pointing to 0x00000000 <NULL>.

I imagine this is an instance where I don't have a full enough understanding of C++, meaning that I could be making a n00b mistake. Or maybe it's some bigger crazy problem. Help?

Update

I've been able to make this work by making mInstruction into a (dumb) pointer:

// in parser, when parsing
mInstructionPtr = new AInstruction(assemblyCode.substr(1), lineNumber);

// elsewhere in AssemblerInstruction.cpp
AssemblerInstruction* AssemblyParser::getInstructionPtr() { return mInstructionPtr; } 

TEST(ParserA, parsesBuiltInConstants)
{
    auto ptr = Parser("@R3", 0).getInstructionPtr();
    AInstruction* a = dynamic_cast<AInstruction*>(ptr);
    EXPECT_EQ(a->getLine(), "R3");
}

However I have trouble implementing it with a unique_ptr: (I'm aware that mInstruction (non-pointer) is redundant, as are two types of pointers. I'll get rid of it later when I clean all this up)

class AssemblyParser
{
public:
    AssemblyParser(std::string assemblyCode, unsigned int lineNumber);
    AssemblerInstruction getInstruction();
    std::unique_ptr<AssemblerInstruction> getUniqueInstructionPtr();
    AssemblerInstruction* getInstructionPtr();

private:
    AssemblerInstruction mInstruction;
    std::unique_ptr<AssemblerInstruction> mUniqueInstructionPtr;
    AssemblerInstruction* mInstructionPtr;
};


// in AssemblyParser.cpp

// in parser as in example above. this works fine.
mUniqueInstructionPtr = make_unique<AInstruction>(assemblyCode.substr(1), lineNumber);

// this doesn't compile!!!
unique_ptr<AssemblerInstruction> AssemblyParser::getUniqueInstructionPtr()
{
    return mUniqueInstructionPtr;
}

In getUniqueInstructionPtr, there is a squiggle under mUniqueInstructionPtr with this error:

'std::unique_ptr<AssemblerInstruction,std::default_delete>::unique_ptr(const std::unique_ptr<AssemblerInstruction,std::default_delete> &)': attempting to reference a deleted function

What!? I haven't declared any functions as deleted or defaulted!

Jonathan Tuzman
  • 11,568
  • 18
  • 69
  • 129

1 Answers1

2

You can not downcast an object to something which doesn't match it's dynamic type. In your code,

 AssemblerInstruction inst = Parser("@R3", 0).getInstruction();

inst has a fixed type, which is AssemblerInstruction. Downcasting it to AInstruction leads to undefined behavior - manifested as crash - because that is not what it is.

If you want your getInstruction to return a dynamically-typed object, it has to return a [smart] pointer to base class, while constructing an object of derived class. Something like that (pseudo code):

 std::unique_ptr<AssemblerInstruction> getInstruction(...) {
      return std::make_unique<AInstruction>(...);
 }

Also, if you see yourself in need of downcasting object based on a value of a class, you are doing something wrong, as you are trying to home-brew polymorphism. Most of the times it does indicate a design flaw, and should instead be done using built-in C++ polymorphic support - namely, virtual functions.

SergeyA
  • 61,605
  • 5
  • 78
  • 137
  • 1
    Worth to mention that even if `getInstruction();` returns `AInstruction` or `CInstruction`, [it was sliced](https://stackoverflow.com/questions/274626/what-is-object-slicing) to `AssemblerInstruction`. I suppose this is where OP's confusion comes from. – Yksisarvinen Jan 15 '21 at 16:34
  • Thanks @Yksisarvinen. This seems like the problem, but that doesn't mean it solves the problem because I don't understand casting/slicing well enough. Sounds like maybe `mInstruction` should be a *pointer* to an `AssemblerInstruction`? Would that prevent slicing? – Jonathan Tuzman Jan 15 '21 at 16:35
  • @JonathanTuzman This might be helpful: https://stackoverflow.com/questions/274626/what-is-object-slicing – NathanOliver Jan 15 '21 at 16:38
  • @JonathanTuzman Yes, seems that Sergey already updated the answer to include that with returning a smart pointer. – Yksisarvinen Jan 15 '21 at 16:39
  • Thanks very much Sergey. I'm about to try implementing this and will mark it as an answer if (when!) it works. I did want to inquire into your last point. My application code never downcasts, it indeed uses the return of `Parser.getInstruction()` to call a virtual `AssemblerInstruction::getMachineCode`. Only in my unit tests, where I want to test what kind of instruction the parser created, completely independent of how an instruction produces its machine code, do I need to downcast to access the otherwise private members of the instructions (which only have getters for testing). Thoughts? – Jonathan Tuzman Jan 15 '21 at 19:04
  • @JonathanTuzman up to you, different people have different expectations from unit tests. Some want them to test as white box, and others want them to test as black box. In my mind, both approaches are valid. – SergeyA Jan 15 '21 at 19:12