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

Add missing MIR_get_global_item #141

Open
wants to merge 1 commit into
base: v0_master
Choose a base branch
from

Conversation

Jasu
Copy link

@Jasu Jasu commented Nov 11, 2020

Hi,

MIR_get_global_item was missing a definition, so I added it according to my best guess. That guess might be way off though, since I'm completely new to MIR. I also did not test if it works with forward definitions or re-exports.

@vnmakarov
Copy link
Owner

Thank you for the pull request. I was not aware that there should be such function (there is no documentation for it in MIR.md). Probably the prototype reflects some my intention to have this function.

As for the pull request, the code is correct but not optimal. The item list can be quite long and we should use the table. I add this function implementation later. Thank you again for letting me know about the missed function.

@Jasu
Copy link
Author

Jasu commented Nov 14, 2020

The item list can be quite long and we should use the table.

Yeah, I actually tried to use the table first, but found out that item_hash uses the pointer to the string, not the string contents (i.e. the cast-to-uint64 in the following code, (uint64_t) MIR_item_name (NULL, it))):

static htab_hash_t item_hash (MIR_item_t it, void *arg) {
  return mir_hash_finish (
    mir_hash_step (mir_hash_step (mir_hash_init (28), (uint64_t) MIR_item_name (NULL, it)),
                   (uint64_t) it->module));
}

I add this function implementation later.

Thanks! I'll manage until then with my implementation.

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.

2 participants