gz-rendering icon indicating copy to clipboard operation
gz-rendering copied to clipboard

Noble warnings: API pure virtual overrides/hidden overrides

Open j-rivero opened this issue 2 months ago • 0 comments

Environment

  • OS Version: 22.04
  • Source or binary build: source, gz-rendering8 branch

Description

When building gz-rendering8 branch the compiler brings a good bunch of warnings. Most of them are related to pure virtual methods overriding other pure virtual methods in base classes. Sometimes the override is not using exactly the same signature. Incorporating the Base class method into the Derived class is easy but will trigger problems with ambiguous calls:

/home/jrivero/code/gz/gz-rendering/test/common_test/ParticleEmitter_TEST.cc:109:20: error: call to member function 'SetMaterial' is ambiguous
  109 |   particleEmitter->SetMaterial(expectedMaterial);
      |   ~~~~~~~~~~~~~~~~~^~~~~~~~~~~
/home/jrivero/code/gz/gz-rendering/include/gz/rendering/Visual.hh:88:28: note: candidate function
   88 |       public: virtual void SetMaterial(MaterialPtr _material,
      |                            ^
/home/jrivero/code/gz/gz-rendering/include/gz/rendering/ParticleEmitter.hh:170:28: note: candidate function
  170 |       public: virtual void SetMaterial(const MaterialPtr _material) = 0;
      |                            ^

So the base class Visual.hh define the method as:

public: virtual void SetMaterial(const std::string &_name, bool _unique = true) = 0;

and the derived class ParticleEmitter.hh has:

public: virtual void SetMaterial(const MaterialPtr _material) = 0;

We can add an override to the Derived class since the signature is not the same. Not sure about how we want to deal with this problem but seems to me like avoiding the ambiguity for the consumers of the classes would be great.

j-rivero avatar Apr 22 '24 18:04 j-rivero