-
-
Notifications
You must be signed in to change notification settings - Fork 789
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
Stack overflow in serde_codegen #494
Comments
serde_codegen 0.8.1 allows setting the @alexcrichton what is the stack size you are using now? I'd like to hardcode it if the |
Thanks! Will play around. Is it possible to get this documented for 8.1? |
|
thanks
ok, that's pretty weird. Since it builds on other systems, it can't be infinite recursion. Can you try building it in release mode? Maybe optimizations will help on your system. Also, run with |
@oli-obk will debug when i get a spare moment. The output from Unless serde_codegen::expand(path, &dst).unwrap(); is invoking the compiler?
^^ thats the file name that it is failing on. |
ok, so with gdb you should be able to execute the |
This is where it fails. Grabbing the stack at the moment. |
long post to follow(switched to lldb because its what i know :))
|
curious, it's not an infinite recursion, but a new sub-expression. I'm guessing it's caused by the following generated code: let mut state =
try!(_serializer . serialize_struct (
"PullRequestInfo" ,
0 + 1 + 1 + 1 + 1 + 1 + 1 + 1 + 1 + 1 + 1 + 1 + 1 + 1 + 1 + 1 + 1 + 1 + 1 + 1
+ 1 + 1 + 1 + 1 + 1 + 1 + 1 + 1 + 1 + 1 + 1 + 1 + 1 + 1 + 1
)); It's the only long chained binary expression. But it still makes no sense that upping the stack limit doesn't work for you. Is We can definitely improve on some of this generated code... But it's going to make the code more complicated to read if we keep adding performance hacks :/ |
|
To satisfy my curiosity dropped a Then ran Outcome is still the same :(
|
oooooh... I'm so stupid. The |
That seems to have fixed the overflow issue. Now i have another issue :) rustc seems to not think pullrequestdata implements Will dig around. |
I think this bug as stands could be closed with some docs :) Perhaps the documentation on https://github.com/serde-rs/serde could be update the build.rs example it currently reads
that could become
with a note underneath that if you experience a error stackoverflow error set the environment variable I am happy to make this change if others are happy? |
Thanks for your help @jaredwy. I updated the setup steps in http://serde.rs/codegen-hybrid.html to use |
Thanks! It is a little strange that code that works on windows is failing on mac post compile though. But i can raise a seperate issue if i find it to be the fault of serde. Thanks for all your help everyone! |
In #503 I changed serde_codegen to use a 16 MB stack by default even if you do not use EDIT: so I removed |
Good to hear! Glad my frustration was fruitful for others :D |
@jaredwy reports a failure building jaredwy/friendlyrust@3eeb035 on OS X 10.10.5 using Rust 1.10.0 and serde_codegen 0.8.0, 0.8.1 and master.
He reports that the same commit builds fine on Windows. It builds fine for me on Linux as well as OS X 10.11.6 using Rust 1.10.0 so there is not much to go on. Possibly related to #229.
The text was updated successfully, but these errors were encountered: