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

Fix ODR violation for plFactory and avoid including it everywhere #1452

Merged
merged 4 commits into from
Aug 8, 2023

Conversation

dgelessus
Copy link
Contributor

Touching the core plCreatable headers, part two.

The private members of plFactory were only conditionally defined in plFactory.h, which is an ODR violation and gives warnings with GCC for example (although apparently it has never caused any issues in practice). I'm not sure why it was done that way (perhaps to reduce header pollution very slightly?), but it seems safe to define everything unconditionally.

Regardless, to avoid pulling in plFactory in many places where it's not needed, I also adjusted plCreatable.h to not automatically include plFactory.h anymore. This required removing a couple of plCreatable methods, but they weren't used anywhere and are trivial to replace.

It was only used by two plCreatable methods: Create and HasDerivedClass.
Both of them are completely unused and trivial to replace.
plClassName::Create() can always be replaced by new plClassName()
(avoiding plFactory entirely) and plClassName::HasDerivedClass(hDer)
can be replaced by plFactory::DerivesFrom(plClassName::Index(), hDer).

This allows removing the plFactory.h include from plCreatable.h - most
users of plCreatable don't need plFactory.
Now that plFactory.h is no longer included *everywhere* via
plCreatable.h, this is less of an issue.

This fixes a violation of the One Definition Rule, which e. g. GCC
warned about (although in practice it didn't cause any issues).
Copy link
Member

@dpogue dpogue left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems good to me.

plFactory::Create(plWhatever::Index()) works just as well as plWhatever::Create() for the esoteric cases that no normal person ever runs into.

@Hoikas Hoikas merged commit 2df0f46 into H-uru:master Aug 8, 2023
@dgelessus dgelessus deleted the plfactory_unprivate branch November 29, 2023 21:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants