Re: clang++ warns

[Andrew Janke]

On 2/15/19 12:20 AM, c. wrote:
> Hi,
>
> Compiling a recent version of the default branch with clang++ I see many many 
> repetitions of the warnings below.
> It is just warnings, I don't know whether this is expected or it is something 
> to worry about, but I thought it might be worth reporting.
>
> CC-ing jwe as this seems related to his new rework of the interpreter.
>
> c.
>
>
>
> In file included from ../octave/libinterp/corefcn/call-stack.h:38:
> ../octave/libinterp/corefcn/stack-frame.h:923:18: warning: 
> 'octave::user_fcn_stack_frame::varval' hides overloaded virtual function 
> [-Woverloaded-virtual]
>      octave_value varval (const symbol_record& sym) const;
>                   ^
> ../octave/libinterp/corefcn/stack-frame.h:831:18: note: hidden overloaded 
> virtual function 'octave::base_value_stack_frame::varval' declared here: type 
> mismatch at 1st
>        parameter ('size_t' (aka 'unsigned long') vs 'const octave::symbol_record 
> &')
>      octave_value varval (size_t data_offset) const
>                   ^
> ../octave/libinterp/corefcn/stack-frame.h:925:19: warning: 
> 'octave::user_fcn_stack_frame::varref' hides overloaded virtual function 
> [-Woverloaded-virtual]
>      octave_value& varref (const symbol_record& sym);
>                    ^
> ../octave/libinterp/corefcn/stack-frame.h:836:19: note: hidden overloaded 
> virtual function 'octave::base_value_stack_frame::varref' declared here: type 
> mismatch at 1st
>        parameter ('size_t' (aka 'unsigned long') vs 'const octave::symbol_record 
> &')
>      octave_value& varref (size_t data_offset)
>                    ^
> ../octave/libinterp/corefcn/stack-frame.h:983:18: warning: 
> 'octave::scope_stack_frame::varval' hides overloaded virtual function 
> [-Woverloaded-virtual]
>      octave_value varval (const symbol_record& sym) const;
>                   ^
> ../octave/libinterp/corefcn/stack-frame.h:831:18: note: hidden overloaded 
> virtual function 'octave::base_value_stack_frame::varval' declared here: type 
> mismatch at 1st
>        parameter ('size_t' (aka 'unsigned long') vs 'const octave::symbol_record 
> &')
>      octave_value varval (size_t data_offset) const
>                   ^
> ../octave/libinterp/corefcn/stack-frame.h:985:19: warning: 
> 'octave::scope_stack_frame::varref' hides overloaded virtual function 
> [-Woverloaded-virtual]
>      octave_value& varref (const symbol_record& sym);
>                    ^
> ../octave/libinterp/corefcn/stack-frame.h:836:19: note: hidden overloaded 
> virtual function 'octave::base_value_stack_frame::varref' declared here: type 
> mismatch at 1st
>        parameter ('size_t' (aka 'unsigned long') vs 'const octave::symbol_record 
> &')
>      octave_value& varref (size_t data_offset)
>                    ^

I'm a little confused by the form of the warnings (the type mismatch 
message, that is), but I think this does indicate a bug: because e.g. 
varval(const symbol_record&) and varval(size_t) are declared virtual in 
the superclass octave::stack_frame, I would really expect them to be 
virtual in all the subclasses as well, so that subclass methods override 
them. The impact of varval and others not being virtual in the 
subclasses is, as I understand it, the dispatched method will end up 
being dependent on the type of the pointer you call it through, instead 
of the type of the pointed-to object.

Let's say you have a octave::user_fcn_stack_frame object. If you store a 
pointer to it in a "octave::user_fcn_stack_frame *f" variable and call 
f->varval(...), then it will call the subclass's 
octave::user_fcn_stack_frame::varval function. But if you store a 
pointer to that same octave::user_fcn_stack_frame object in a 
"octave::stack_frame *f" variable and call f->varval(...), then the 
superclass's octave::stack_frame::varval function will be called 
instead. This is usually not what you want.

If the subclass methods are intended to override the superclass methods 
in the normal C++ way, then they need to be virtual in the subclasses as 
well as the superclasses. "virtual" doesn't just mean "this method can 
be overridden"; it means "this method participates in an override 
relationship", either as overrider or overridee.

Cheers,
Andrew