-
Notifications
You must be signed in to change notification settings - Fork 4k
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
THRIFT-5819: use latest rustc version for rustlib #3045
base: master
Are you sure you want to change the base?
Conversation
a9f332e
to
294e371
Compare
@@ -2818,7 +2816,7 @@ mod tests { | |||
copy_write_buffer_to_read_buffer!(o_prot); | |||
|
|||
let read_double = i_prot.read_double().unwrap(); | |||
assert!(read_double - double < std::f64::EPSILON); | |||
assert!((read_double - double).abs() < f64::EPSILON); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🤣 great catch
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
+1 LGTM
still pending some compile err that i didn't have time to look into. not sure if someone familiar with the rust stack can help. |
This warning (which is turned into an error by diff --git a/lib/rs/src/lib.rs b/lib/rs/src/lib.rs
index 84c1f9b71..2f6018810 100644
--- a/lib/rs/src/lib.rs
+++ b/lib/rs/src/lib.rs
@@ -53,7 +53,7 @@
//! [tutorial]: https://github.com/apache/thrift/tree/master/tutorial/rs
#![crate_type = "lib"]
-#![doc(test(attr(allow(unused_variables), deny(warnings))))]
+#![doc(test(attr(allow(unused_variables, dead_code), deny(warnings))))]
#![deny(bare_trait_objects)]
// NOTE: this macro has to be defined before any modules. See: |
@jimexist the remaining issues can be fixed with this diff: diff --git a/compiler/cpp/src/thrift/generate/t_rs_generator.cc b/compiler/cpp/src/thrift/generate/t_rs_generator.cc
index 780c8fd29..e336077e1 100644
--- a/compiler/cpp/src/thrift/generate/t_rs_generator.cc
+++ b/compiler/cpp/src/thrift/generate/t_rs_generator.cc
@@ -749,8 +749,8 @@ void t_rs_generator::render_const_value(t_type* ttype,
} else if (ttype->is_enum()) {
f_gen_ << "{" << '\n';
indent_up();
- f_gen_ << indent() << to_rust_type(ttype) << "::try_from(" << tvalue->get_integer()
- << ").expect(\"expecting valid const value\")" << '\n';
+ f_gen_ << indent() << to_rust_type(ttype) << "::from(" << tvalue->get_integer()
+ << ")" << '\n';
indent_down();
f_gen_ << indent() << "}";
} else if (ttype->is_struct() || ttype->is_xception()) {
diff --git a/test/rs/src/bin/test_client.rs b/test/rs/src/bin/test_client.rs
index fd3a18550..801ccc4b5 100644
--- a/test/rs/src/bin/test_client.rs
+++ b/test/rs/src/bin/test_client.rs
@@ -252,10 +252,7 @@ fn make_thrift_calls(
info!("testi64");
// try!(verify_expected_result(thrift_test_client.test_i64(-8651829879438294565),
// -8651829879438294565));
- verify_expected_result(
- thrift_test_client.test_i64(i64::min_value()),
- i64::min_value(),
- )?;
+ verify_expected_result(thrift_test_client.test_i64(i64::MIN), i64::MIN)?;
info!("testDouble");
verify_expected_result( |
thanks for the pointer. fwiw you can just submit a new PR - i don't use rust lib myself so i am not that much motivated to update this (nor be able to actually test the changes). |
I've tested it locally, running all the commands that the CI does, and I think that should resolve all the issues. I can submit another PR, but it may take a few days before I can put that up. |
I've opened #3085 to succeed this PR. |
use latest rustc version for rustlib
[skip ci]
anywhere in the commit message to free up build resources.