Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Noble warnings: API pure virtual overrides/hidden overrides #991

Open
j-rivero opened this issue Apr 22, 2024 · 0 comments
Open

Noble warnings: API pure virtual overrides/hidden overrides #991

j-rivero opened this issue Apr 22, 2024 · 0 comments
Labels
bug Something isn't working

Comments

@j-rivero
Copy link
Contributor

j-rivero commented Apr 22, 2024

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 j-rivero added the bug Something isn't working label Apr 22, 2024
@azeey azeey moved this from Inbox to To do in Core development May 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
Status: To do
Development

No branches or pull requests

1 participant