Skip to content
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

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

Closed
guusw opened this issue Mar 22, 2022 · 3 comments
Closed

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

guusw opened this issue Mar 22, 2022 · 3 comments

Comments

@guusw
Copy link

guusw commented Mar 22, 2022

#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
@djc
Copy link
Member

djc commented Mar 23, 2022

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)
 }

@guusw
Copy link
Author

guusw commented Mar 23, 2022

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

djc pushed a commit that referenced this issue May 12, 2022
First attempt to fix this issue for wasm32-unknown-* targets by
changing visibility of functions not to leak crate-private types.

Ensure the default toolchain is used for the wasm_unknown CI test.
esheppa pushed a commit to esheppa/chrono that referenced this issue May 13, 2022
…tope#684)

First attempt to fix this issue for wasm32-unknown-* targets by
changing visibility of functions not to leak crate-private types.

Ensure the default toolchain is used for the wasm_unknown CI test.
@pitdicker
Copy link
Collaborator

I did not try to test this, but believe it can be closed because all these methods were removed in #1072.

@djc djc closed this as completed Jun 4, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants