[Potential Bug introduced in v4.25] Calling virtual method in AbstractPainterColor constructor
Our team updated a project from v4.22 to v4.25 this week and our clang-tidy scan flagged the following warning message in the constructor of every class that had a "Painter" member variable ("Painter" is aliased in our solution as "using Painter = touchgfx::PainterRGB565;").
AbstractPainterColor.hpp (External\Middlewares\ST\touchgfx\framework\touchgfx\widgets\canvas)
Call to virtual method `AbstractPainterColor::setColor` during construction bypasses virtual dispatch [clang-analyzer-optin.cplusplus.VirtualCall, -warnings-as-errorss] GCC [Ln45, Col9]
Container.cpp[Ln 29, Col12]: Calling default constructor for 'PainterRGB565'
PainterRGB565.hpp[Ln 43, Col 36]: Calling default constructor for 'AbstractPainterColor'
AbstractPainterColor.hpp[Ln 45, Col 9]: Call to virtual method 'AbstractPainterColor::setColor' during construction bypasses virtual dispatch
In looking at the warning message, it's not necessarily a problem, depending on the intent.
- Explanation of warning: https://clang.llvm.org/docs/analyzer/checkers.html#optin-cplusplus-virtualcall
- Rules for virtual methods in constructors: https://en.cppreference.com/w/cpp/language/virtual#During_construction_and_destruction
Depending on the intent, it appears this is fine, but here's some noteables:
- AbstractPainterColor's constructor changed from explicitly calling AbstractPainterColor::setColor(...) to simply setColor(...). (introducing the warning)
- PainterRGB565 also defines a setColor(...) method (and inherits AbstractPainterColor).
- As long as there is not a presumption that AbstractPainterColor's constructor is calling PainterRGB565::setColor(...), I think it is fine to suppress the warning, which we are doing.
- If for some reason that is incorrect, either we have a misunderstanding on how things work or the behavior is not as was desired with this change (or perhaps is undefined) due to the fact that "the more-derived class does not yet exist" per cppreference.com.
Perhaps this is fine (and/or my understanding of the message is incorrect), but it might warrant a closer look by the TouchGFX development team if the actual behavior is not the desired behavior in this case.