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

bytes!() is now hard to use in a lot of cases due to lifetime issues #11641

Closed
lilyball opened this issue Jan 18, 2014 · 7 comments · Fixed by #14275
Closed

bytes!() is now hard to use in a lot of cases due to lifetime issues #11641

lilyball opened this issue Jan 18, 2014 · 7 comments · Fixed by #14275
Labels
A-lifetimes Area: Lifetimes / regions

Comments

@lilyball
Copy link
Contributor

With #3511 landed, the lifetime of bytes!() is now problematic. Specifically, the following used to work and now fails:

let b = match true {
    true => bytes!("test"),
    false => unreachable!()
};

The lifetime of bytes!("test") is now limited to that branch of the match, so it doesn't live long enough to be assigned to b.

I'm not sure if this means lifetimes need to change. The other option is that bytes!() needs to return a &'static [u8]. Unfortunately, the only way to do that right now is to make it equivalent to {static b: &'static [u8] = &[...]; b }, which makes it no longer usable as the value of a constant (e.g. static foo: &'static [u8] = bytes!("foo")).

I'm thinking the right solution is to make it legal for me to say &'static [1u8]. I can type that right now, but the 'static lifetime seems to be wholly ignored, as I get the exact same error with the following as I do with the bytes!() example above:

let b = match true {
    true => &'static [1u8],
    false => unreachable!()
};

If &'static [1u8] becomes a valid expression with the 'static lifetime, then bytes!() could start evaluating to that and everything should work as expected. Furthermore, it would be nice (but perhaps not required) if &[1u8], where all elements are compile-time constants, would implicitly be given the 'static lifetime.

@lilyball
Copy link
Contributor Author

/cc @nikomatsakis

@nikomatsakis
Copy link
Contributor

Yeah, this is kind of annoying. I worked around it in one specific case by having bytes! expand as follows:

{
    static bytes: &'static [u8] = ...;
    bytes
}

I certainly think one should be allowed to write a concrete lifetime as part of a slice or borrow expression, and 'static ought to be legal -- we'll obviously have to impose more stringent constant-related rules. In the short term, we could adjust bytes! to expand in the pattern I showed above. What was unclear to me was whether this is sufficient workaround, or whether we'll want something more general.

Interestingly, even if we used full inference, the expression you gave above would be illegal unless we introduce a special case for static byte strings, because it generates a conditional temporary (only on one arm of the match) and we can't handle the cleanup for such a case without zeroing, which we plan to remove (well, I guess we could also have a special case for temporary types [like [u8]) that do not require drop glue, but I'd rather not).

@nikomatsakis
Copy link
Contributor

Perhaps the simplest fix for now is to adjust the bytes! macro itself to expand to a static item as I showed. I should probably have done that more generally but I was ready to land that expletive branch already.

@sfackler
Copy link
Member

@nikomatsakis Won't that prevent bytes! from being able to expand to a static?

static FOO: &'static [u8] = {
    static F: &'static [u8] = &'static [10u8];
    F
};
test.rs:1:29: 4:2 error: constant contains unimplemented expression type
test.rs:1 static FOO: &'static [u8] = {
test.rs:2     static F: &'static [u8] = &'static [10u8];
test.rs:3     F
test.rs:4 };
error: aborting due to previous error
task 'rustc' failed at 'explicit failure', /home/sfackler/rust/rust/src/libsyntax/diagnostic.rs:75
task '<main>' failed at 'explicit failure', /home/sfackler/rust/rust/src/librustc/lib.rs:448

@lilyball
Copy link
Contributor Author

@nikomatsakis I already adjusted bytes!() like that and @sfackler is correct, it prevents static FOO: &'static [u8] = bytes!("foo"), which is definitely already used in our codebase.

@nikomatsakis
Copy link
Contributor

Ah, unfortunate. (I actually think we should allow statics to include nested blocks with other statics, but I guess that's a separate bug) Well, I'm not sure what's the easiest fix in that case, I guess just to fix the semantics of &'static [11, 22, 33] so that it guarantees static data. I'm not 100% sure what's implied by such a fix: it doesn't seem too hard ;)

@huonw
Copy link
Member

huonw commented Jan 27, 2014

cc #11640

lilyball added a commit to lilyball/rust that referenced this issue May 18, 2014
Change `bytes!()` to return

    {
        static BYTES: &'static [u8] = &[...];
        BYTES
    }

This gives it the `'static` lifetime, whereas before it had an rvalue
lifetime. Until recently this would have prevented assigning `bytes!()`
to a static, as in

    static FOO: &'static [u8] = bytes!(1,2,3);

but rust-lang#14183 fixed it so blocks are now allowed in constant expressions
(with restrictions).

Fixes rust-lang#11641.
bors added a commit that referenced this issue May 18, 2014
Change `bytes!()` to return

    {
        static BYTES: &'static [u8] = &[...];
        BYTES
    }

This gives it the `'static` lifetime, whereas before it had an rvalue
lifetime. Until recently this would have prevented assigning `bytes!()`
to a static, as in

    static FOO: &'static [u8] = bytes!(1,2,3);

but #14183 fixed it so blocks are now allowed in constant expressions
(with restrictions).

Fixes #11641.
flip1995 pushed a commit to flip1995/rust that referenced this issue Feb 26, 2024
…r=Jarcho

Allow negative literals in `redundant_guards`

changelog: none
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-lifetimes Area: Lifetimes / regions
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants