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

Warn when Signal::global is used in a const global #1976

Closed
ealmloff opened this issue Feb 24, 2024 · 1 comment
Closed

Warn when Signal::global is used in a const global #1976

ealmloff opened this issue Feb 24, 2024 · 1 comment
Labels
signals Related to the signals crate tweak Small changes to improve experience

Comments

@ealmloff
Copy link
Member

Specific Demand

If Signal::global is used in const global instead of a static global, multiple states will be created. Consts effectively copy the const value into every place where the const is used.

const SIGNAL: GlobalSignal<u32> = Signal::global(|| 0);

// This will create a signal
SIGNAL.read(); // -> Signal::global(|| 0).read()

// This will create a completely different signal
SIGNAL.read(); // -> Signal::global(|| 0).read()

static STATIC_SIGNAL: GlobalSignal<u32> = Signal::global(|| 0);

// This just references the existing signal if a signal already exists in the global context
STATIC_SIGNAL.read();

Implement Suggestion

We should try to prevent this at compile time or if we cannot prevent it at compile time, warn the user with tracing.

OnceCell faces a similar issue. It might be worth looking into their discussions/issues: matklad/once_cell#224, rust-lang/rust#40543

@ealmloff ealmloff added tweak Small changes to improve experience signals Related to the signals crate labels Feb 24, 2024
@jkelleyrtp
Copy link
Member

I believe this was fixed in #2258 since we now use panic locations as keys which are more resilient to merging even in const contexts.

Despite this being fixed, when you try to use mutative methods on const signals, rust will warn you and tell you to swap it to static.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
signals Related to the signals crate tweak Small changes to improve experience
Projects
None yet
Development

No branches or pull requests

2 participants