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

? or % can't be used with o!(), looks like regression #248

Open
netvl opened this issue Dec 27, 2019 · 7 comments
Open

? or % can't be used with o!(), looks like regression #248

netvl opened this issue Dec 27, 2019 · 7 comments

Comments

@netvl
Copy link
Contributor

netvl commented Dec 27, 2019

It seems that the logic in this PR: #170 has been reverted somehow, because I now get exactly those errors which are described in that PR when I try to use ? or % with o!():


error[E0277]: `*mut (dyn std::ops::Fn() + 'static)` cannot be shared between threads safely
   --> src/server/mail/watcher.rs:755:20
    |
755 |         logger.new(o!("part_path" => ?p))
    |                    ^^^^^^^^^^^^^^^^^^^^^ `*mut (dyn std::ops::Fn() + 'static)` cannot be shared between threads safely
    |
    = help: within `[std::fmt::ArgumentV1<'_>]`, the trait `std::marker::Sync` is not implemented for `*mut (dyn std::ops::Fn() + 'static)`
    = note: required because it appears within the type `std::marker::PhantomData<*mut (dyn std::ops::Fn() + 'static)>`
    = note: required because it appears within the type `core::fmt::Void`
    = note: required because it appears within the type `&core::fmt::Void`
    = note: required because it appears within the type `std::fmt::ArgumentV1<'_>`
    = note: required because it appears within the type `[std::fmt::ArgumentV1<'_>]`
    = note: required because of the requirements on the impl of `std::marker::Send` for `&[std::fmt::ArgumentV1<'_>]`
    = note: required because it appears within the type `std::fmt::Arguments<'_>`
    = note: required because it appears within the type `slog::SingleKV<std::fmt::Arguments<'_>>`
    = note: required because it appears within the type `(slog::SingleKV<std::fmt::Arguments<'_>>, ())`
    = note: required because of the requirements on the impl of `slog::SendSyncRefUnwindSafeKV` for `(slog::SingleKV<std::fmt::Arguments<'_>>, ())`
    = note: required by `slog::OwnedKV`
    = note: this error originates in a macro outside of the current crate (in Nightly builds, run with -Z external-macro-backtrace for more info)

And indeed, the kv! macro now uses __slog_builtin(@format_args) instead of whatever is implemented in #170, which in turn implements the pre-#170 approach of using format_args.

@dpc
Copy link
Collaborator

dpc commented Dec 29, 2019

I thought there was a test and all. Can you check which version of slog are you using exactly?

I'd appreciate some help. Not sure when I'll be able to dig into this.

@nazar-pc
Copy link

nazar-pc commented Jan 2, 2020

2.5.2 all the way down to 2.4.0 has this problem.
2.3.3 failed with some other error, so I'm not sure about it.

@dcormier
Copy link
Contributor

dcormier commented Jul 9, 2021

Still an issue on 2.7.0.

@panicbit
Copy link
Contributor

It looks like the original fix got reverted in 4695580 and fcf3efe.
This also got documented in the changelog.

I would be glad to fix this issue again, but it would be helpful to know why the change was considered broken back then. Otherwise I feel like the risk of wasting my time is too great.

@dpc
Copy link
Collaborator

dpc commented Jan 12, 2022

🤷 No idea, sorry. (but PR with some tests for it would be awesome.)

@ppershing
Copy link

It looks like the original fix got reverted in 4695580 and fcf3efe. This also got documented in the changelog.

I would be glad to fix this issue again, but it would be helpful to know why the change was considered broken back then. Otherwise I feel like the risk of wasting my time is too great.

I believe this is the relevant report of breakage: #176 (comment)

@Techcable
Copy link
Member

Techcable commented Oct 11, 2024

In addition to fixing this, we're going to have to add a bunch of tests so this never happens again.

I've considered switching to a procedural macro, but that would likely require depending on syn.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

7 participants