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

pointerof lib external #5398

Merged
merged 1 commit into from Dec 20, 2017
Merged

Conversation

larubujo
Copy link
Contributor

fixes #4845

first pr so maybe not good. tried with libc environ, worked.

implement mostly by copy other code. LibType has add_var for externals with primitive "external_var_get". so for pointerof call lookup_var if pointerof(LibType.var). only if looks like that, not if has args or block or named args.

codegen not sure. in semantic bind to var, which stored in dependencies. only one dependency, so that dependency is the external. codegen_primitive_external_var_get has code for get pointer of external. so use that, refactor a bit. call check_c_fun not sure, i think thats if external var has fun type, but didn't try. but if works with regular vars already improvement over existing code.

@RX14
Copy link
Contributor

RX14 commented Dec 18, 2017

@ysbaddaden can you test this PR with your original usecase?

@Papierkorb
Copy link
Contributor

Papierkorb commented Dec 20, 2017

Could you add a spec that:

  1. make sure the returned pointer is not NULL (As sanity check)
  2. interact with the pointer (Set the value through the pointer and read it back from the variable would suffice)

@larubujo
Copy link
Contributor Author

updated. found one can test c code interaction, so use that

y = ptr.value

x + y
), &.to_i.should eq(11))
Copy link
Contributor

Choose a reason for hiding this comment

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

I'd prefer this spec to be more robust: we should check the value of LibFoo.external_var after setting it too. I think there should be a way to return multiple values from the test. Does returning a tuple work?

Copy link
Contributor

Choose a reason for hiding this comment

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

Actually use a bool to coalesce the multiple tests.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

we should check the value of LibFoo.external_var after setting it too

test does that:

ptr.value = 10

y = ptr.value

not understand

no way to test tuple. maybe you write test, i copy?

Copy link
Contributor

Choose a reason for hiding this comment

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

It checks ptr.value, but not LibFoo.external_var.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

good point. updated

@RX14 RX14 added this to the Next milestone Dec 20, 2017
@RX14 RX14 merged commit 7791254 into crystal-lang:master Dec 20, 2017
Sija referenced this pull request in ysbaddaden/gc Dec 20, 2017
Constants are initialized into the DATA and BSS sections, so the
collector must mark from these roots, in addition to fiber stacks.

Since we can't access the address of an extern, we rely on a small
C helper script to access some linker symbols (`__data_start`,
`__bss_start` and `_end`).
@RX14 RX14 modified the milestones: 0.24.1, Next Dec 24, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Can't take address of extern lib variables
4 participants