Hi Johan, this is the actual warning:
../../cppify/src/sp-item.cpp:1267:11: warning: 'this' pointer cannot be null in well-defined C++ code; pointer may be assumed to always convert to true [-Wundefined-bool-conversion] if (!(this && (SP_IS_TEXT(this) || SP_IS_USE(this)))) { ^~~~ ~~
../../cppify/src/sp-lpe-item.cpp:215:10: warning: 'this' pointer cannot be null in well-defined C++ code; pointer may be assumed to always convert to true [-Wundefined-bool-conversion] if (!this) { ~^~~~
I read this as "I will optimize this check away anyway." I'm not sure whether that's the case.
Regards, Markus
-----Ursprüngliche Nachricht----- Von: Johan Engelen [mailto:jbc.engelen@...2592...] Gesendet: Mittwoch, 23. Juli 2014 20:54 An: Markus Engel Cc: Inkscape-Devel Betreff: Code cleanup in trunk
Hi Markus, Thanks for the code cleanups in trunk. One comment: please be careful in removing nullptr checks in trunk. Although they look unnecessary, we really don't want to (re-)add bugs. In rev 13460, you removed a this == nullptr check. It's terrible code, I agree. The problem is that some other bad code may still rely on that check. I'm not saying that's the case for that particular piece of code, but I'd rather not remove such checks in trunk until after release.
Small tip for people bughunting, "this" in a class method can be nullptr. A call through nullptr to non-virtual method will happily pass. Because we changed code like f(SPFoo *g) { if (!g) return; to Foo::f() { if (!this) return; you may see a check on "this" in the code. If you are sure that all callers make sure the object is not nullptr, you can remove the check in experimental.
thanks, Johan