Code review comment for lp:~liampwhite/inkscape/inkscape-cppify

Revision history for this message
Johan Engelen (johanengelen) wrote :

> @Johan:
>
> I'll do what you ask. There seems to be this small problem, though:
> Many files are forward declaring "struct Application", and according to ISO:
>
> The address of an object of incomplete type can be taken, but if the
> complete type of that object is a class type that declares operator&() as a
> member function, then the behavior is undefined (and no diagnostic is
> required).
>
> So those will all need to be changed (forward declarations into #include[s]).

This is still better than exposing operator& at all.

A number of case of "struct Application" should be removed:
Probably should be removed. Stuff like document-undo.cpp, line 115:
Inkscape::DocumentUndo::resetKey( Inkscape::Application * /*inkscape*/, SPDesktop * /*desktop*/, GObject *base )
should not have Application* as argument at all. It's even unused!
In document.h, the fwd decl is not used.
In dialog-events.h, "sp_transientize_callback ( Inkscape::Application *inkscape, " does not use that argument either...
err.. all cases of Application appearing in fwd declarations or as function arguments are actually completely unused.......!

All this passing around of pointers to Application seems pretty bad, if Application is meant to be a singleton.

« Back to merge proposal