Skip to content

Conversation

@dhthwy
Copy link
Contributor

@dhthwy dhthwy commented Nov 12, 2025

may resolve #5517

I believe this does the trick. Just added some wrappers so that DataIdentity.h doesn't have to include LuaWrapper.h which in turn includes lua.h.

sadly ninja -d stats didn't report any measurable difference in build times.

void field_error_(lua_State *state, int index, const char* err, const char* mode)
{
DFHack::LuaWrapper::field_error(state, index, err, mode);
}
Copy link
Member

@ab9rf ab9rf Nov 13, 2025

Choose a reason for hiding this comment

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

These introduce potential pessimizations by preventing the compiler from inlining these at their call sites. Since at least two of these functions are used in potentially high-velocity code paths, I'm reluctant to approve this without runtime profiling

that said, i just tested dfhack compiled with LTCG and it seems to work on Windows, which would obviate this concern if we can confirm that using LTCG doesn't break dfhack, and someone can figure out how to do the same thing for linux users

};
#endif

type_identity* lua_touserdata_(lua_State* state);
Copy link
Member

Choose a reason for hiding this comment

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

are the trailing underscores necessary? can we not just fully-qualify the names when needed (or would that require changing too many places)?

Copy link
Contributor Author

@dhthwy dhthwy Nov 14, 2025

Choose a reason for hiding this comment

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

These are forwarding functions (intended for DataIdentity only) used by DataIdentity.c so that we can include LuaWrapper.h which in turn includes lua.h in the DataIdentity TU instead of the header.

the real lua_touserdata() takes another argument too.

_wrapper or something similar is another alternative. I wasn't sure what name to give it, hence the underscores.

I don't think they're necessary?

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.

including VTableInterpose.h forces a dependency on lua for no good reason

3 participants