chrono icon indicating copy to clipboard operation
chrono copied to clipboard

Can't leak crate-private type in stub.rs

Open guusw opened this issue 2 years ago • 2 comments

#661 was just merged however the stubs implementation doesn't seem to have changed pub to pub(super) causing the following on the wasm32-unknown-emscripten target:

error[E0446]: crate-private type `Tm` in public interface
  --> /home/runner/.cargo/git/checkouts/chrono-cf67fff630261eb4/d6d02ac/src/sys/stub.rs:68:1
   |
68 | pub fn time_to_local_tm(sec: i64, tm: &mut Tm) {
   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ can't leak crate-private type
   |
  ::: /home/runner/.cargo/git/checkouts/chrono-cf67fff630261eb4/d6d02ac/src/sys/mod.rs:77:1
   |
77 | pub(crate) struct Tm {
   | -------------------- `Tm` declared as crate-private

error[E0446]: crate-private type `Tm` in public interface
  --> /home/runner/.cargo/git/checkouts/chrono-cf67fff630261eb4/d6d02ac/src/sys/stub.rs:73:1
   |
73 | pub fn utc_tm_to_time(tm: &Tm) -> i64 {
   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ can't leak crate-private type
   |
  ::: /home/runner/.cargo/git/checkouts/chrono-cf67fff630261eb4/d6d02ac/src/sys/mod.rs:77:1
   |
77 | pub(crate) struct Tm {
   | -------------------- `Tm` declared as crate-private

error[E0446]: crate-private type `Tm` in public interface
  --> /home/runner/.cargo/git/checkouts/chrono-cf67fff630261eb4/d6d02ac/src/sys/stub.rs:77:1
   |
77 | pub fn local_tm_to_time(tm: &Tm) -> i64 {
   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ can't leak crate-private type
   |
  ::: /home/runner/.cargo/git/checkouts/chrono-cf67fff630261eb4/d6d02ac/src/sys/mod.rs:77:1
   |
77 | pub(crate) struct Tm {
   | -------------------- `Tm` declared as crate-private

guusw avatar Mar 22 '22 10:03 guusw

Sorry for breaking this, and thanks for reporting it! How are you testing this? Would you be able to submit a PR that adds something to our CI workflow to cover this case?

The actual changes to fix this are probably simple enough:

diff --git a/src/sys/stub.rs b/src/sys/stub.rs
index 9172a85..616b52f 100644
--- a/src/sys/stub.rs
+++ b/src/sys/stub.rs
@@ -65,16 +65,16 @@ fn tm_to_time(tm: &Tm) -> i64 {
         + s
 }
 
-pub fn time_to_local_tm(sec: i64, tm: &mut Tm) {
+pub(super) fn time_to_local_tm(sec: i64, tm: &mut Tm) {
     // FIXME: Add timezone logic
     time_to_tm(sec, tm);
 }
 
-pub fn utc_tm_to_time(tm: &Tm) -> i64 {
+pub(super) fn utc_tm_to_time(tm: &Tm) -> i64 {
     tm_to_time(tm)
 }
 
-pub fn local_tm_to_time(tm: &Tm) -> i64 {
+pub(super) fn local_tm_to_time(tm: &Tm) -> i64 {
     // FIXME: Add timezone logic
     tm_to_time(tm)
 }

djc avatar Mar 23 '22 03:03 djc

I'll look into making a PR and testing it.

guusw avatar Mar 23 '22 09:03 guusw

I did not try to test this, but believe it can be closed because all these methods were removed in https://github.com/chronotope/chrono/pull/1072.

pitdicker avatar Jun 04 '23 09:06 pitdicker