-1

I have the following code:

struct Operation {
    public :
        OperationName name;
};

struct FilterOperation : Operation {
    FilterName filter;
    std::list<std::string> params;
};

The OperationName and FilterName are enums listing all the different names of each Operation and Filter.

In a for each loop going though all the Operations, I want to downcast a Operation to a FilterOperation:

std::list<Operation> operations
for (Operation op : operations) {
switch (op.name) {
    case o_filter :
        std::cout  << dynamic_cast<FilterOperation*>(&op)->filter << std::endl;
    }
}

Obviously the dynamic_cast doesn't work here:

parser.cpp:154:90: error: cannot dynamic_cast ‘& op’ (of type ‘struct Operation*’) to type ‘struct FilterOperation*’ (source type is not polymorphic)
 "Op: Filter, Name: " << filterStrings[dynamic_cast<FilterOperation*>(&op)->filter] 

I actually tried adding a virtual function to it, but that doesn't resolve my actual issue (I can't figure out how to downcast here properly)

Estefunny
  • 43
  • 1
  • 10
  • 5
    but - you have a list of operations; they're not filteroperations. Even if they were based on filter operations, you sliced them and lost anything filter based about them – UKMonkey Jun 07 '18 at 11:07
  • https://stackoverflow.com/questions/15114093/getting-source-type-is-not-polymorphic-when-trying-to-use-dynamic-cast – jspcal Jun 07 '18 at 11:10
  • Are you able to stock a FilterOperation object in the list of Operation objects in the first place? – jszpilewski Jun 07 '18 at 11:11
  • I thought the list accept the base types, but could also accept the child structs, could I change the list to achieve this behavior? – Estefunny Jun 07 '18 at 11:12
  • @jszpilewski: well, the compiler didn't complain about this one: FilterOperation op; operations.push_back (op); – Estefunny Jun 07 '18 at 11:14
  • @Estefunny Yes, but slicing happened. https://stackoverflow.com/questions/274626/what-is-object-slicing – Sebastian Redl Jun 07 '18 at 11:24
  • You should use a list of pointers, smart pointers or `std::ref` to `Operation`. That way, no slicing will occur. – Serge Ballesta Jun 07 '18 at 11:27
  • @Estefunny, So you stocked Operation objects being a copy of subset of the data in FilterOperation. In the end if you want to be able to cast to FilterOperation* keep pointers in the list too. – jszpilewski Jun 07 '18 at 11:28

3 Answers3

3

This is undefined behavior.

op is an Operation. Not a pointer or reference to a Operation, lesser to a FilterOperation. So &op is clearly not a FilterOperation*.

From cppreference.com on dynamic_cast:

dynamic_cast < new_type > ( expression )      

If the cast is successful, dynamic_cast returns a value of type new_type. If the cast fails and new_type is a pointer type, it returns a null pointer of that type.

So clearly, dynamic_cast<FilterOperation*>(&op) is a null pointer, and dereferencing it is UB.

Community
  • 1
  • 1
YSC
  • 38,212
  • 9
  • 96
  • 149
1

Your intuition about adding a virtual function should have resolved the compiler error for you. Are you sure you didn't get some other error?

In any case, because you are dealing with instances of objects rather than pointers, this will never work. Your list consists of Operation objects, not FilterOperation objects. If you want to insert FilterOperation objects then you need a list of pointers (or preferably, shared_ptr) instead of storing by value:

std::list<Operation*> operations
for (Operation* op : operations) {
switch (op->name) {
    case o_filter :
        std::cout  << dynamic_cast<FilterOperation*>(op)->filter << std::endl;
    }
}

Additionally, I suspect you misunderstand what switch() will do for character strings. It will probably not do what you want, and you need an if statement instead.

flodin
  • 5,215
  • 4
  • 26
  • 39
  • Im switching on enum values, not strings – Estefunny Jun 07 '18 at 11:28
  • You should add a test whether the dynamic_cast returns `nullptr` because dereferencing it is UB – Serge Ballesta Jun 07 '18 at 11:29
  • @SergeBallesta Yes but I don't want to muddle the answer with stuff that's not directly related to what Estefunny is trying to understand. I think their confusion stems from not understanding object slicing in C++; they probably come from a Java/.Net background. Knowing to check for nullptr isn't the issue. – flodin Jun 07 '18 at 11:38
  • Ok, but you should at least mention that in a additional line in your post. I agree it is not the question, but your code cannot not be used directly because of the UB – Serge Ballesta Jun 07 '18 at 11:50
1

Using stl containers you use the copy constructor and operator = of Operation. These method copy only the Operation from FilterOperation not the whole structure.

To solve the issue, you should use std::list<Operation*> or even better, std::list<std::shared_ptr<Operation>> instead of std::list<Operation>

that way you won't copy the Operation to the list, just the pointer...

In addition, you must add a virtual destructor to Operation struct, otherwise you'll have a memory leak because the list in the derived class (FilterOperation) won't be freed when you delete an Operation.

SHR
  • 7,940
  • 9
  • 38
  • 57