-
Notifications
You must be signed in to change notification settings - Fork 2
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
Add relations to HIRM in an order respecting their base_relations #197
Conversation
cxx/hirm.cc
Outdated
if (std::holds_alternative<T_clean_relation>(relation)) { | ||
this->add_relation(prng, name, relation); | ||
HIRM::HIRM(const T_schema& _schema, std::mt19937* prng) { | ||
printf("in hirm\n"); |
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.
remove?
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.
Done.
|
||
if (!added_a_relation) { | ||
printf("Detected loop in schema\n"); | ||
std::exit(1); |
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.
We've used asserts everywhere else, let's stick with 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.
assert's are removed in optimized mode, so I've tried not to use them in places like this where continuing will lead to bad behavior (an infinite loop in this case). grep says that there are 19 such places in the codebase.
} | ||
|
||
if (!added_a_relation) { | ||
printf("Detected loop in schema\n"); |
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.
I think it'd be helpful to print the relation name.
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 is outside of the loop over relations, so there is no single relation name to print.
Fixes #196