Restructure internal library dependencies (Fix #383) #794
Conversation
Hoikas
force-pushed
the
moar-link-libs
branch
from
cdfb418
to
881e8a1
Compare
Hoikas
force-pushed
the
moar-link-libs
branch
3 times, most recently
from
e2cef9d
to
d50cc1d
Compare
Hoikas
force-pushed
the
moar-link-libs
branch
from
d50cc1d
to
ced5c2d
Compare
|
This has been rebased to merge cleanly after the great overriding. |
Hoikas
force-pushed
the
moar-link-libs
branch
2 times, most recently
from
d96e99a
to
4117f43
Compare
|
Rebased to merge cleanly after the good de-tarray-izing. |
There was a problem hiding this comment.
[...] moving virtual function definitions into source files from headers
I'm curious why? There's nothing wrong with inlining a virtual function, and modern compilers can even apply devirtualization to an inline virtual function if it knows it can correctly assume the applicable override.
I was hoping to reduce the size of the dependency graph from headers. In some cases it was fairly successful - by moving IO virtual functions into cpp files, I was able to, for example, avoid other headers pulling in all of st_format from plFileSystem via hsStream. Given the use pattern of |
Hoikas
force-pushed
the
moar-link-libs
branch
2 times, most recently
from
a128bd9
to
21c2b99
Compare
Ah, that makes sense... I had thought you were trying to remove every virtual function from header files, which seemed excessive to me. |
|
Yeah, I think any small function that's just a few lines long is fine in a header, especially if it's something we actively want inlined. It's a judgement call based off the length of the function and if that function needed specific or large |
Sources/Plasma/FeatureLib/pfConditional/plAnimationEventConditionalObject.cpp
Outdated
Show resolved
Hide resolved
Hoikas
force-pushed
the
moar-link-libs
branch
2 times, most recently
from
cdae253
to
44e7d64
Compare
This fixes the majority of H-uru#383. In order to do so, I began to reorder some of the include directives as specified in H-uru#788 to facillitate easier inspection of the dependencies. The work is not complete at all. Further, I broke several dependencies where trivially possible by changing uses of derived classes to base classes and moving virtual function definitions into source files from headers. The diff is unpleasant, I know :(
Always link against ye olde CoreLib and thou shalt be fine, sayeth Hoikas.
Co-authored-by: Michael Hansen <zrax0111@gmail.com>
I mean, the only thing that uses OpenGL currently is a single include in a stub file. Hardwiring the names of what appears to be Linux libraries breaks macOS CI.
... And writing in a reader, apparently.
Hoikas
force-pushed
the
moar-link-libs
branch
from
44e7d64
to
a35cf6e
Compare
Co-authored-by: Michael Hansen <zrax0111@gmail.com>
This completely fixes #383. In order to do so, I began to reorder some of the include directives as specified in #788 to facilitate
easier inspection of the dependencies. The work on that proposal is not complete. Further, I broke several dependency loops where trivially possible by changing uses of derived classes to base classes and moving virtual function definitions into source files from headers.
This depends on #791 so I can test on Linux.I suggest looking at some of the resulting CMakeLists, eg plClient's, and large include blocks, eg pfConsole, to understand the goal. Sorry for how gnarly the diff is 😢 .