-
Notifications
You must be signed in to change notification settings - Fork 12.7k
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
fastffi attribute is sometimes ignored or incorrectly assigned #6660
Comments
I'm not entirely clear what's going on here. Both functions appear to have the same type but the testcase says it's testing them being different. Can you clarify? |
Woah, I forgot about this bug. This is an interesting test case. It has nothing to do with fastffi. In fact, i'm not sure why it crashes, it kind of .. looks like an LLVM bug? But something fishy is definitely going on. Here is an updated test:
On my branch for #8535, this code crashes when
However, when I dump the LLVM output with
There is only one call here at all? Anyway, an interesting bug. (note that I am running on 32 bit linux, btw, which may be relevant) |
Should it be casting the whole function as opposed to just the argument in the second call? |
Oh, wait, you're right, I misread that LLVM code, there are in fact two calls. I'd say it... looks about right, actually. Yes, it should be casting the function. |
This is no longer used. |
Cleanup path-to-local checks changelog: none It seemed like too much ceremony going on to check if an expression matches a variable. So I created two util functions `path_to_local(Expr) -> Option<HirId>` and `path_to_local_id(Expr, HirId) -> bool` to make this easier, and used them wherever applicable. I changed logic in a few places to use `HirId` instead of `Symbol` where it was easy to do so. I believe this is more correct and may even fix some bugs. I also removed some calls to `qpath_res`. This is not needed if you are only looking for a `Res::Local`. As a note, I wanted to name the util functions in a way that encourages understanding of the HIR.
The following test, originally written for #3678, results in a segfault because insufficient stack is allocated (at least it does on the branch I plan to land that fixes #3678).
Paging @pcwalton, maybe you could take a look (once the PR for #3678 lands?)
The text was updated successfully, but these errors were encountered: