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

chore(appender): fix compile on PowerPC #1508

Closed
wants to merge 2 commits into from

Conversation

dzvon
Copy link
Contributor

@dzvon dzvon commented Aug 16, 2021

Motivation

Currently, tracing-appender crate cannot be compiled on PowerPC platform. Because

PowerPC and MIPS platforms with 32-bit pointers do not have AtomicU64 or AtomicI64 types.

quote from std library docs. (https://doc.rust-lang.org/std/sync/atomic/index.html#portability)

Solution

Change AtomicU64 to AtomicUsize.

Signed-off-by: Dezhi Wu <wu543065657@163.com>
@nagisa
Copy link
Contributor

nagisa commented Aug 16, 2021

If we do this, we should ensure that we don't overflow the counter back to 0 (replace fetch_add with a version that saturates…?)

@hawkw hawkw mentioned this pull request Sep 19, 2021
@hawkw hawkw closed this in #1675 Oct 22, 2021
hawkw added a commit that referenced this pull request Oct 22, 2021
This branch is @dzvon's PR #1508, with the following changes:

* Add a newtype wrapping the error counter to hide that it's internally
  an `Arc<AtomicUsize>`. This would allow us to make additional changes
  to the implementation without potentially causing breaking changes.

* Use saturating arithmetic when incrementing the counter to avoid
  wrapping to 0 on overflows. This is more likely to be an issue on
  32-bit platforms.

This is a breaking change that will be released as part of 
`tracing-appender` 0.2.

Closes #1508

Description from @dzvon's original PR:

## Motivation

Currently, tracing-appender crate cannot be compiled on PowerPC
platform. Because

> PowerPC and MIPS platforms with 32-bit pointers do not have
> `AtomicU64` or `AtomicI64` types.

quote from std library docs.
(https://doc.rust-lang.org/std/sync/atomic/index.html#portability)

## Solution

Change `AtomicU64` to `AtomicUsize`.

Co-authored-by: Dezhi Wu <wu543065657@163.com>
hawkw added a commit that referenced this pull request Oct 22, 2021
This branch is @dzvon's PR #1508, with the following changes:

* Add a newtype wrapping the error counter to hide that it's internally
  an `Arc<AtomicUsize>`. This would allow us to make additional changes
  to the implementation without potentially causing breaking changes.

* Use saturating arithmetic when incrementing the counter to avoid
  wrapping to 0 on overflows. This is more likely to be an issue on
  32-bit platforms.

This is a breaking change that will be released as part of 
`tracing-appender` 0.2.

Closes #1508

Description from @dzvon's original PR:

## Motivation

Currently, tracing-appender crate cannot be compiled on PowerPC
platform. Because

> PowerPC and MIPS platforms with 32-bit pointers do not have
> `AtomicU64` or `AtomicI64` types.

quote from std library docs.
(https://doc.rust-lang.org/std/sync/atomic/index.html#portability)

## Solution

Change `AtomicU64` to `AtomicUsize`.

Co-authored-by: Dezhi Wu <wu543065657@163.com>
hawkw added a commit that referenced this pull request Oct 22, 2021
This branch is @dzvon's PR #1508, with the following changes:

* Add a newtype wrapping the error counter to hide that it's internally
  an `Arc<AtomicUsize>`. This would allow us to make additional changes
  to the implementation without potentially causing breaking changes.

* Use saturating arithmetic when incrementing the counter to avoid
  wrapping to 0 on overflows. This is more likely to be an issue on
  32-bit platforms.

This is a breaking change that will be released as part of 
`tracing-appender` 0.2.

Closes #1508

Description from @dzvon's original PR:

## Motivation

Currently, tracing-appender crate cannot be compiled on PowerPC
platform. Because

> PowerPC and MIPS platforms with 32-bit pointers do not have
> `AtomicU64` or `AtomicI64` types.

quote from std library docs.
(https://doc.rust-lang.org/std/sync/atomic/index.html#portability)

## Solution

Change `AtomicU64` to `AtomicUsize`.

Co-authored-by: Dezhi Wu <wu543065657@163.com>
hawkw added a commit that referenced this pull request Oct 22, 2021
This branch is @dzvon's PR #1508, with the following changes:

* Add a newtype wrapping the error counter to hide that it's internally
  an `Arc<AtomicUsize>`. This would allow us to make additional changes
  to the implementation without potentially causing breaking changes.

* Use saturating arithmetic when incrementing the counter to avoid
  wrapping to 0 on overflows. This is more likely to be an issue on
  32-bit platforms.

This is a breaking change that will be released as part of 
`tracing-appender` 0.2.

Closes #1508

Description from @dzvon's original PR:

## Motivation

Currently, tracing-appender crate cannot be compiled on PowerPC
platform. Because

> PowerPC and MIPS platforms with 32-bit pointers do not have
> `AtomicU64` or `AtomicI64` types.

quote from std library docs.
(https://doc.rust-lang.org/std/sync/atomic/index.html#portability)

## Solution

Change `AtomicU64` to `AtomicUsize`.

Co-authored-by: Dezhi Wu <wu543065657@163.com>
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

Successfully merging this pull request may close these issues.

2 participants