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

Borrowed pointers inside enums #3166

Closed
Emm opened this issue Aug 9, 2012 · 9 comments
Closed

Borrowed pointers inside enums #3166

Emm opened this issue Aug 9, 2012 · 9 comments

Comments

@Emm
Copy link

Emm commented Aug 9, 2012

The following code fails with:

argparse.rs:31:24: 34:21 error: mismatched types: expected `core::option::option<&str>` but found `core::option::option<&selfstr>` (reference with lifetime &self as defined on the block at 21:45 does not necessarily outlive reference with lifetime & as defined on the block at 21:45)
argparse.rs:31                 banner: match self.banner {
argparse.rs:32                         none => { none }
argparse.rs:33                         some(value) => { some(value) }
argparse.rs:34                     },
error: aborting due to previous error
mod argparse {
    use std;

    import std::map;
    import either::{either, left, right};

    struct Flag {
        name: &str;
        desc: &str;
        short_name: option<&str>;
        max_count: uint;
        banner: option<&str>;
        mut value: uint;
    }

    fn flag(name: &str, desc: &str) ⟶  Flag {
        Flag { name: name, desc: desc, short_name: none, max_count: 1, banner: none, value: 0 }
    }

    impl Flag {
        fn short_name(self, s: &str) ⟶  Flag {
            let new_banner : option<&str> = match self.banner {
                    none =>  { none }
                    some(value) =>  { some(value) }
                };
            Flag {
                name: self.name,
                desc: self.desc,
                short_name: some(s),
                max_count: self.max_count,
                banner: match self.banner {
                        none =>  { none }
                        some(value) =>  { some(value) }
                    },
                value: self.value
            }
        }
    }
}

fn main () {
    let f : argparse::Flag = argparse::flag(&"flag", &"My flag");
}

I would expect it to compile, given that the lifetime of the value inside the option will be the same as the lifetime of the other fields.

@nikomatsakis
Copy link
Contributor

I believe the problem is that the field short_name is given the string s, which is only valid during the function call to short_name() itself. In other words, all the lifetimes are not the same. You can make the compile like so:

mod argparse {
    use std;

    import std::map;
    import either::{either, left, right};

    struct Flag {
        name: &str;
        desc: &str;
        short_name: option<&str>;
        max_count: uint;
        banner: option<&str>;
        mut value: uint;
    }

    fn flag(name: &str, desc: &str) -> Flag {
        Flag { name: name, desc: desc, short_name: none, max_count: 1, banner: none, value: 0 }
    }

    impl Flag {
        fn short_name(self, s: &self/str) -> Flag/&self {
            let new_banner : option<&str> = match self.banner {
                    none => { none }
                    some(value) => { some(value) }
                };
            Flag {
                name: self.name,
                desc: self.desc,
                short_name: some(s),
                max_count: self.max_count,
                banner: match self.banner {
                        none => { none }
                        some(value) => { some(value) }
                    },
                value: self.value
            }
        }
    }
}

fn main () {
    let f : argparse::Flag = argparse::flag(&"flag", &"My flag");
}

all the changes are in the declaration of short_name(), which becomes:

        fn short_name(self, s: &self/str) -> Flag/&self {

The first change is that s becomes &self/str instead of &str. Basically this means "I need a str that lives as long as the borrowed pointers within self". &str just means "lives as long as this function call".

The return type also has to change to Flag/&self. This notation is not well publicized or known, but it means "an instance of Foo containing borrowed pointers with lifetime &self". As before, the default is to return an instance of Flag containing borrowed pointers that are valid only for the call itself.

@Emm
Copy link
Author

Emm commented Aug 9, 2012

Thanks for the detailed explanation and workaround. However, it seems to me that the actual issue is how the borrowed pointer inside self.banner is handled, as it is treated differently from the other fields.

If you take this implementation of short_name() (this is the one I should have posted earlier, instead of the version with 'match'), this fails with the "mismatched type" error:

        fn short_name(self, s: &str) ⟶  Flag {
            Flag {
                name: self.name,
                desc: self.desc,
                short_name: some(s),
                max_count: self.max_count,
                banner: self.banner,
                value: self.value
            }
        }

However, this works:

        fn short_name(self, s: &str) ⟶  Flag {
            Flag {
                name: self.name,
                desc: self.desc,
                short_name: some(s),
                max_count: self.max_count,
                banner: none,
                value: self.value
            }
        }

One would expect to be able to copy optional fields the same way as mandatory fields. Is it an issue with rust expecting non-wrapped borrowed pointers to have a lifetime at least equal to that of their container, but not so with borrowed pointers wrapped in option/either? (Not that I have tested with either but there is no reason the same issues does not come up).

@nikomatsakis
Copy link
Contributor

The field banner is not inconsistent. What's happening is that the type checker is inferring, based on the types given in short_name(), that you want to construct an instance of Flag where the borrowed pointers within have the lifetime &. This is the lifetime parameter given by the caller: it lasts as long as the current call.

In the first case, which reports an error, you have a region pointer in short_name, which has the lifetime &, and a second region pointer in banner (self.banner). This has a different lifetime so an error is reported.

In the second case, the field banner is assigned none, which is a constant, and hence consistent with any lifetime, so no error is reported.

@Emm
Copy link
Author

Emm commented Aug 10, 2012

I understand that. What seems odd to me is that the other fields are also borrowed pointers: I would expect the other fields, like name: &str to be treated the same as banner: option<&str>. This is what I find inconsistent. The compiler complains specifically about the banner field because it's an enum containing a borrowed pointer, while fields containing borrowed pointers without enums (like name) cause no issue, even though, as a programmer, I clearly intend both pointers to have the same lifetime.

@nikomatsakis
Copy link
Contributor

This is a more general problem with reporting of type error messages, I suppose. If you wrote something like this:

struct foo<T> { a: T; b: T; }

fn main() {
    foo { a: 3, b: "hi" };
}

You get an error reporting for the field b, even though you clearly meant for the types of a and b to be the same. So what is happening here is that the compiler has inferred based on the name field what the lifetime of the other fields should be, and then it found that the latter fields (namely banner) were inconsistent with that. (In fact the situation is completely analogous to the type parameter; there is in fact an implicit lifetime parameter on the type whose value is being inferred)

I agree it would be nice if, upon encountering a type error, the compiler displayed all mutually inconsistent items. In fact I think there is an issue open on this point.

@Emm
Copy link
Author

Emm commented Aug 11, 2012

Sorry if I'm being a bit dense here. It's not the explanation of the error which I have an issue with, it's the fact that I do not understand why the region pointer in banner has a different lifetime from the rest. In spite of the fact that the borrowed pointer in banner is wrapped inside option, I expect it to have the same lifetime as, say, desc.

Let me try another example. I'm going to henceforth banish all option fields from Flag, and attempt to create a new flag by passing &s to it, and copying the rest of the borrowed pointers from the old object. If I understand it right, it should not work without using the &self/str type for s: the lifetime of s, whether it's assigned to an option field or not, still needs to be at least equal to the lifetime of the object we are copying, correct?

Let's find out:

mod argparse {
    use std;

    import std::map;
    import either::{either, left, right};

    struct Flag {
        name: &str;
        desc: &str;
        max_count: uint;
        mut value: uint;
    }

    fn flag(name: &str, desc: &str)  ->  Flag {
        Flag { name: name, desc: desc, max_count: 1, value: 0 }
    }

    impl Flag {
        fn set_desc(self, s: &str) ->  Flag {
            Flag {
                name: self.name,
                desc: s,
                max_count: self.max_count,
                value: self.value
            }
        }
    }
}

fn main () {
    let f : argparse::Flag = argparse::flag(~"flag", ~"My flag");
    let updated_flag = f.set_desc(~"My new flag");
    assert updated_flag.desc == "My new flag";
}

Shockingly, not only it compiles, but the assertion works!

@nikomatsakis
Copy link
Contributor

Hmm, this seems like a bug. I do not think this should compile, because the lifetime of self.name is &self and the lifetime of s is the lifetime parameter &, and there is no relationship between them.

@Emm
Copy link
Author

Emm commented Aug 11, 2012

Ok, this makes sense, thanks for taking time to discuss the issue.

@nikomatsakis
Copy link
Contributor

No problem, thanks for uncovering a bug!

saethlin pushed a commit to saethlin/rust that referenced this issue Nov 17, 2023
reallocarray shim linux/freebsd support proposal.
celinval pushed a commit to celinval/rust-dev that referenced this issue Jun 4, 2024
Bumps [tests/perf/s2n-quic](https://github.com/aws/s2n-quic) from
`9730578` to `1436af7`.
<details>
<summary>Commits</summary>
<ul>
<li><a
href="https://github.com/aws/s2n-quic/commit/1436af712b6e73edc11640dc7c3cae23e456c0a8"><code>1436af7</code></a>
ci: Remove neqo from required resumption test clients (<a
href="https://redirect.github.com/aws/s2n-quic/issues/2191">#2191</a>)</li>
<li><a
href="https://github.com/aws/s2n-quic/commit/c0bcef639de0e2a6a89202e84c9933d99d431047"><code>c0bcef6</code></a>
build: remove --cfg s2n_quic_unstable (<a
href="https://redirect.github.com/aws/s2n-quic/issues/2190">#2190</a>)</li>
<li><a
href="https://github.com/aws/s2n-quic/commit/fed54a59dcfdbc70e7c3c2d4b1cf3c4991ad4403"><code>fed54a5</code></a>
feat(s2n-quic-platform): make message methods public (<a
href="https://redirect.github.com/aws/s2n-quic/issues/2189">#2189</a>)</li>
<li>See full diff in <a
href="https://github.com/aws/s2n-quic/compare/9730578c0d562d80bbbe663161b3a5408ed3116c...1436af712b6e73edc11640dc7c3cae23e456c0a8">compare
view</a></li>
</ul>
</details>
<br />


Dependabot will resolve any conflicts with this PR as long as you don't
alter it yourself. You can also trigger a rebase manually by commenting
`@dependabot rebase`.

[//]: # (dependabot-automerge-start)
[//]: # (dependabot-automerge-end)

---

<details>
<summary>Dependabot commands and options</summary>
<br />

You can trigger Dependabot actions by commenting on this PR:
- `@dependabot rebase` will rebase this PR
- `@dependabot recreate` will recreate this PR, overwriting any edits
that have been made to it
- `@dependabot merge` will merge this PR after your CI passes on it
- `@dependabot squash and merge` will squash and merge this PR after
your CI passes on it
- `@dependabot cancel merge` will cancel a previously requested merge
and block automerging
- `@dependabot reopen` will reopen this PR if it is closed
- `@dependabot close` will close this PR and stop Dependabot recreating
it. You can achieve the same result by closing it manually
- `@dependabot show <dependency name> ignore conditions` will show all
of the ignore conditions of the specified dependency
- `@dependabot ignore this major version` will close this PR and stop
Dependabot creating any more for this major version (unless you reopen
the PR or upgrade to it yourself)
- `@dependabot ignore this minor version` will close this PR and stop
Dependabot creating any more for this minor version (unless you reopen
the PR or upgrade to it yourself)
- `@dependabot ignore this dependency` will close this PR and stop
Dependabot creating any more for this dependency (unless you reopen the
PR or upgrade to it yourself)


</details>

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.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

No branches or pull requests

2 participants