-
Notifications
You must be signed in to change notification settings - Fork 12.9k
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
check_static_recursion: Handle foreign items #18279
Conversation
Thanks! Could you add a test for this as well? |
Also, I think I saw an issue about this awhile ago, is this in relation to an open issue? If so, could you put |
I just updated the text to describe the situation. I'm still trying to remember where I encountered this bug. |
This will fix #18360 |
Thanks for tracking that down @bkoropoff! Could you add that test case @bgamari? |
@bkoropoff good catch. @alexcrichton sure, it looks like that ticket should have enough to compose a test. |
0000a4f
to
2a3be29
Compare
@alexcrichton there is now a test case. @jakub- thanks for pointing this out. @alexcrichton which would you like to merge? |
Unfortunately the build-bots seemed to have trouble with my test case (seems to be the same as your own) and I'm not sure how to deal with it to be honest. Here's the error from here:
It seems to try to link with the non-existant static for some reason. I've tried thinking of solutions but I just can't think of anything, I'm sorry. |
@whataloadofwhat very odd. @alexcrichton unless you see any further issues perhaps we should just merge my branch? |
I think that this is the same test as #17973 (which will probably fail for the same reason). You'll need to provide an actual definition of the symbol somehow to get it to link on windows I think. You should be able to get by with creating an auxiliary crate with a |
@whataloadofwhat, are you ok closing in favor of this for now? |
2a3be29
to
9f10653
Compare
@alexcrichton how does this look? It passes on my box but unfortunately I have no Windows machine to test on. |
9f10653
to
7f9db07
Compare
@alexcrichton alright, let's try this one then. |
7f9db07
to
c00a884
Compare
@alexcrichton could you give this one a try; I made the |
c00a884
to
b9251cd
Compare
@alexcrichton I'm not sure why this |
Note to self: |
Ah yes it seems like you found it, be sure to blame @steveklabnik for any troubles you're having :) |
…chton I just found this patch which at some point solved a problem I encountered. Unfortunately I apparently dropped it before I managed to write a test case. I'll try to dig up the code that triggered the issue.
I just found this patch which at some point solved a problem I encountered. Unfortunately I apparently dropped it before I managed to write a test case. I'll try to dig up the code that triggered the issue.