Fix #6031 #8906
Fix #6031 #8906
Conversation
|
Thanks for fixing this! I think that this may want to wait until #8880 is merged, though, because You're right, though, in that there are currently no tests for the C++ code, but once that pull request is merged you can write this in rust and test it as well! |
| @@ -34,9 +34,11 @@ struct log_directive { | |||
| const size_t max_log_level = 255; | |||
| const size_t default_log_level = log_err; | |||
|
|
|||
| const char* log_levels [] = {"err", "warn", "info", "debug", 0 }; | |||
|
Now that the other half has landed, this should be good to go for a rebase now. |
|
(this is now ready for review) |
| ]; | ||
| unsafe { | ||
| do "crate1::mod1".to_c_str().with_ref |ptr| { | ||
| let entry= &ModEntry {name: ptr, log_level: &mut 0}; |
There was a problem hiding this comment.
These tests are actually pretty unsafe. You're creating &mut 0 but there's no guarantee about the lifetime of the 0. Could you move the 0 into an outer variable to ensure that a pointer to it outlasts the construction of ModEntry?
There was a problem hiding this comment.
I also just noticed that this may be a problem in the surrounding tests as well, would you mind updating those as well?
There was a problem hiding this comment.
Surely this is a huge bug in rust's type system if the 0 is allowed here and the lifetime is incorrect. I.e. it either lasts long enough or the compiler will complain?
|
Everything looks good to me! Just style nits here and there and one safety issue in the tests. otherwise r+ |
|
OK, I think I've addressed all the comments. Thanks for working with me on this pull request. |
This is a patch to fix #6031. I didn't see any tests for the C++ library code, so I didn't write a test for my changes. Did I miss something, or are there really no tests?
…Jarcho remove `large_enum_variant` suggestion for `Copy` types Replaces the (erroneous) suggestion on `large_enum_variant` for `Copy` types by a note. This fixes rust-lang#8894. --- changelog: none
This is a patch to fix #6031. I didn't see any tests for the C++ library code, so I didn't write a test for my changes. Did I miss something, or are there really no tests?