-
Notifications
You must be signed in to change notification settings - Fork 52
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
Code tree search structs refactor + lazily expanded flat terms #543
Conversation
…ng version in term code trees
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fine by me, see comments. As usual I trust you with the details of what you're actually doing but it looks generally sensible and there's less hold-on-to-your-hat pointer manouevring going on, so I'm happy about that!
Indexing/CodeTree.cpp
Outdated
@@ -35,6 +35,9 @@ | |||
namespace Indexing | |||
{ | |||
|
|||
#define GET_CONTAINING_OBJECT(ContainingClass,MemberField,object) \ | |||
reinterpret_cast<ContainingClass*>(reinterpret_cast<size_t>(object)-offsetof(ContainingClass,MemberField)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks dubious to me as I don't think size_t
is guaranteed to be big enough. What I think you want to do it cast to char *
and then do the arithmetic on that.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You're right, this is also implemented on Linux. The main problem is with char*
we need to const_cast
as well, because it is called with both const and non-const arguments. Let me know which one do you think is better.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Two macros, one for each case? Could also then be static_cast
, I think.
3984a05
to
e0a6b0c
Compare
Thanks for the fixes/improvements, @mezpusz! I'm happy apart from the |
No new assertion violations and slightly improves performance. I think it's ready to go! |
OK! I'll wait to see how/if @mezpusz wants to fix |
Two main changes in this pull request:
CodeTree::SearchStruct
is refactored avoiding duplication of subclasses. Raw array containing value-target pairs is replaced with STL containers for better and easier handling.FUN_UNEXPANDED
toFlatTerm
in which case only the tag, term pointer and number of subentries is filled out first. A call onFlatTerm::Entry::expand
fills out the same entries for the arguments. The point is that we expand tha flat term lazily which in some cases would be a bottleneck when theFlatTerm
is not even used.Additionally, added some basic printing for code trees.