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 ordinals not being applied to return types of symbols #1162

Merged
merged 5 commits into from
Oct 29, 2024
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
78 changes: 44 additions & 34 deletions lib/ast/definition.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -172,6 +172,45 @@ SymbolMap kore_definition::get_overloads() const {
return transitive_closure(overloads);
}

static void process_sort_ordinal(
kore_sort *sort,
std::unordered_map<kore_composite_sort, uint32_t, hash_sort> &sorts,
std::vector<kore_composite_sort *> &all_sorts, uint32_t &next_sort) {
// We use a work list to ensure that parametric sorts get ordinals
// that are greater than the ordinals of any of their parameters.
// This invariant is usefull for serialization purposes, and given
// that all parametric sorts are statically known, it is sound to
// assign ordinals to them in such a topological order.
std::stack<std::pair<kore_composite_sort *, bool>> worklist;
auto *ctr = dynamic_cast<kore_composite_sort *>(sort);
worklist.push(std::make_pair(ctr, false));

while (!worklist.empty()) {
auto *sort_to_process = worklist.top().first;
bool params_processed = worklist.top().second;
worklist.pop();

if (!sorts.contains(*sort_to_process)) {
if (!params_processed) {
// Defer processing this sort until its parameter sorts have
// been processed.
worklist.push(std::make_pair(sort_to_process, true));
for (auto const &param_sort : sort_to_process->get_arguments()) {
auto *param_ctr
= dynamic_cast<kore_composite_sort *>(param_sort.get());
worklist.push(std::make_pair(param_ctr, false));
}
continue;
}

sorts.emplace(*sort_to_process, next_sort++);
all_sorts.push_back(sort_to_process);
}

sort_to_process->set_ordinal(sorts[*sort_to_process]);
}
}

// NOLINTNEXTLINE(*-function-cognitive-complexity)
void kore_definition::preprocess() {
get_subsorts();
Expand Down Expand Up @@ -286,40 +325,11 @@ void kore_definition::preprocess() {
for (auto *symbol : entry.second) {
if (symbol->is_concrete()) {
for (auto const &sort : symbol->get_arguments()) {
// We use a work list to ensure that parametric sorts get ordinals
// that are greater than the ordinals of any of their parameters.
// This invariant is usefull for serialization purposes, and given
// that all parametric sorts are statically known, it is sound to
// assign ordinals to them in such a topological order.
std::stack<std::pair<kore_composite_sort *, bool>> worklist;
auto *ctr = dynamic_cast<kore_composite_sort *>(sort.get());
worklist.push(std::make_pair(ctr, false));

while (!worklist.empty()) {
auto *sort_to_process = worklist.top().first;
bool params_processed = worklist.top().second;
worklist.pop();

if (!sorts.contains(*sort_to_process)) {
if (!params_processed) {
// Defer processing this sort until its parameter sorts have
// been processed.
worklist.push(std::make_pair(sort_to_process, true));
for (auto const &param_sort :
sort_to_process->get_arguments()) {
auto *param_ctr
= dynamic_cast<kore_composite_sort *>(param_sort.get());
worklist.push(std::make_pair(param_ctr, false));
}
continue;
}

sorts.emplace(*sort_to_process, next_sort++);
all_sorts_.push_back(sort_to_process);
}

sort_to_process->set_ordinal(sorts[*sort_to_process]);
}
process_sort_ordinal(sort.get(), sorts, all_sorts_, next_sort);
}
if (symbol->get_sort()->is_concrete()) {
process_sort_ordinal(
symbol->get_sort().get(), sorts, all_sorts_, next_sort);
Copy link
Collaborator

Choose a reason for hiding this comment

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

I noticed that I missed processing the return sort in my implementation of the work list. However checking back at the original code, I don't think this was there either. So could you explain why this is needed? Is it the case that it missing was a bug even before we added the work list implementation?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It was a bug that was there before your changes were made, yes. The issue is that formal parameters that appear only in the return sort of the symbol (e.g. the second parameter of inj) were not getting their ordinal set and thus were getting serialized incorrectly.

}
if (!instantiations.contains(*symbol)) {
instantiations.emplace(*symbol, next_symbol++);
Expand Down
Binary file modified test/binary/test_rich_header.kore.ref
Binary file not shown.
Loading