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

Tracking Issue for #![feature(offset_of)] #106655

Closed
5 of 7 tasks
WaffleLapkin opened this issue Jan 9, 2023 · 77 comments
Closed
5 of 7 tasks

Tracking Issue for #![feature(offset_of)] #106655

WaffleLapkin opened this issue Jan 9, 2023 · 77 comments
Labels
C-tracking-issue Category: An issue tracking the progress of sth. like the implementation of an RFC F-offset_of `#![feature(offset_of)]` T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.

Comments

@WaffleLapkin
Copy link
Member

WaffleLapkin commented Jan 9, 2023

Feature gates tracked by this issue:

This is a tracking issue for the offset_of! macro which evaluates to a constant
containing the offset in bytes of a field inside some type (rust-lang/rfcs#3308).

Public API

// core::mem

pub macro offset_of($Container:ty, $field:tt $(,)?) {
    // ...implementation defined...
}

Steps / History

Possible future extensions / work

Unresolved Questions

  • None yet.

Footnotes

  1. https://std-dev-guide.rust-lang.org/feature-lifecycle/stabilization.html

@WaffleLapkin WaffleLapkin added T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. C-tracking-issue Category: An issue tracking the progress of sth. like the implementation of an RFC F-offset_of `#![feature(offset_of)]` labels Jan 9, 2023
@tgross35

This comment was marked as resolved.

bors added a commit to rust-lang-ci/rust that referenced this issue Apr 22, 2023
Add offset_of! macro (RFC 3308)

Implements rust-lang/rfcs#3308 (tracking issue rust-lang#106655) by adding the built in macro `core::mem::offset_of`. Two of the future possibilities are also implemented:

* Nested field accesses (without array indexing)
* DST support (for `Sized` fields)

I wrote this a few months ago, before the RFC merged. Now that it's merged, I decided to rebase and finish it.

cc `@thomcc` (RFC author)
bjorn3 pushed a commit to bjorn3/rust that referenced this issue Apr 29, 2023
Add offset_of! macro (RFC 3308)

Implements rust-lang/rfcs#3308 (tracking issue rust-lang#106655) by adding the built in macro `core::mem::offset_of`. Two of the future possibilities are also implemented:

* Nested field accesses (without array indexing)
* DST support (for `Sized` fields)

I wrote this a few months ago, before the RFC merged. Now that it's merged, I decided to rebase and finish it.

cc `@thomcc` (RFC author)
Darksonn pushed a commit to Darksonn/linux that referenced this issue May 2, 2023
This macro is used to compute the offset of a field in a struct.

This commit enables two unstable features that are necessary for using
the macro in a constant. However, this is not a problem as the macro
will become available from the Rust standard library soon [1]. The
unstable features can be disabled again once that happens.

The macro in this patch does not support sub-fields. That is, you cannot
write `offset_of!(MyStruct, field.sub_field)` to get the offset of
`sub_field` with `field`'s type being a struct with a field called
`sub_field`. This is because `field` might be a `Box<SubStruct>`, which
means that you would be trying to compute the offset to something in an
entirely different allocation. There's no easy way to fix the current
macro to support subfields, but the version being added to the standard
library should support it, so the limitation is temporary and not a big
deal.

Link: rust-lang/rust#106655 [1]
Signed-off-by: Wedson Almeida Filho <[email protected]>
Co-developed-by: Alice Ryhl <[email protected]>
Signed-off-by: Alice Ryhl <[email protected]>
@est31
Copy link
Member

est31 commented May 4, 2023

Some things that could get tests:

  • offset_of!(Self, ...) works, if and only if it's inside an impl block.
  • the return type of offset_of!() is usize: some code that tries to use it as usize -> works. some code that tries to use it as u8/isize -> fails.

Edit: #111665

flip1995 pushed a commit to flip1995/rust that referenced this issue May 5, 2023
Add offset_of! macro (RFC 3308)

Implements rust-lang/rfcs#3308 (tracking issue rust-lang#106655) by adding the built in macro `core::mem::offset_of`. Two of the future possibilities are also implemented:

* Nested field accesses (without array indexing)
* DST support (for `Sized` fields)

I wrote this a few months ago, before the RFC merged. Now that it's merged, I decided to rebase and finish it.

cc `@thomcc` (RFC author)
Dylan-DPC added a commit to Dylan-DPC/rust that referenced this issue May 9, 2023
Implement builtin # syntax and use it for offset_of!(...)

Add `builtin #` syntax to the parser, as well as a generic infrastructure to support both item and expression position builtin syntaxes. The PR also uses this infrastructure for the implementation of the `offset_of!` macro, added by rust-lang#106934.

cc `@petrochenkov` `@DrMeepster`

cc rust-lang#110680 `builtin #` tracking issue
cc rust-lang#106655 `offset_of!` tracking issue
Darksonn pushed a commit to Darksonn/linux that referenced this issue May 10, 2023
This macro is used to compute the offset of a field in a struct.

This commit enables two unstable features that are necessary for using
the macro in a constant. However, this is not a problem as the macro
will become available from the Rust standard library soon [1]. The
unstable features can be disabled again once that happens.

The macro in this patch does not support sub-fields. That is, you cannot
write `offset_of!(MyStruct, field.sub_field)` to get the offset of
`sub_field` with `field`'s type being a struct with a field called
`sub_field`. This is because `field` might be a `Box<SubStruct>`, which
means that you would be trying to compute the offset to something in an
entirely different allocation. There's no easy way to fix the current
macro to support subfields, but the version being added to the standard
library should support it, so the limitation is temporary and not a big
deal.

Link: rust-lang/rust#106655 [1]
Signed-off-by: Wedson Almeida Filho <[email protected]>
Co-developed-by: Alice Ryhl <[email protected]>
Signed-off-by: Alice Ryhl <[email protected]>
nbdd0121 pushed a commit to nbdd0121/linux that referenced this issue May 10, 2023
This macro is used to compute the offset of a field in a struct.

This commit enables two unstable features that are necessary for using
the macro in a constant. However, this is not a problem as the macro
will become available from the Rust standard library soon [1]. The
unstable features can be disabled again once that happens.

The macro in this patch does not support sub-fields. That is, you cannot
write `offset_of!(MyStruct, field.sub_field)` to get the offset of
`sub_field` with `field`'s type being a struct with a field called
`sub_field`. This is because `field` might be a `Box<SubStruct>`, which
means that you would be trying to compute the offset to something in an
entirely different allocation. There's no easy way to fix the current
macro to support subfields, but the version being added to the standard
library should support it, so the limitation is temporary and not a big
deal.

Link: rust-lang/rust#106655 [1]
Signed-off-by: Alice Ryhl <[email protected]>
Co-Developed-by: Wedson Almeida Filho <[email protected]>
Signed-off-by: Wedson Almeida Filho <[email protected]>
@Amanieu
Copy link
Member

Amanieu commented May 14, 2023

Small nit on the docs: I don't think "stable" is the right word to use for describing the output on non-repr(C) structs since this causes confusion with Rust's concept of stable APIs.

@rfcbot fcp merge

@rfcbot
Copy link

rfcbot commented May 14, 2023

Team member @Amanieu has proposed to merge this. The next step is review by the rest of the tagged team members:

Concerns:

Once a majority of reviewers approve (and at most 2 approvals are outstanding), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up!

See this document for info about what commands tagged team members can give me.

@rfcbot rfcbot added proposed-final-comment-period Proposed to merge/close by relevant subteam, see T-<team> label. Will enter FCP once signed off. disposition-merge This issue / PR is in PFCP or FCP with a disposition to merge it. labels May 14, 2023
@m-ou-se
Copy link
Member

m-ou-se commented May 14, 2023

Looks like it has an issue: 0.0 doesn't work right now for nested tuple fields.

@rfcbot rfcbot added final-comment-period In the final comment period and will be merged soon unless new substantive objections are raised. and removed proposed-final-comment-period Proposed to merge/close by relevant subteam, see T-<team> label. Will enter FCP once signed off. labels May 14, 2023
@rfcbot
Copy link

rfcbot commented May 14, 2023

🔔 This is now entering its final comment period, as per the review above. 🔔

@WaffleLapkin
Copy link
Member Author

@Amanieu do you suggest to stabilize offset_of? Could you clarify what is the FCP for? It seems a bit too soon, as this feature was just recently implemented.

@m-ou-se
Copy link
Member

m-ou-se commented May 15, 2023

This FCP is to confirm that libs-api is happy with committing to this macro existing forever with this interface. That seems refreshingly uncontroversial in this case.

Whether the implementation is robust/bug free enough is a bit of a separate question, and it might be worth delaying stabilization for that.

If you think there is value in having more time for the interface/api itself, please share your thoughts on that!

@WaffleLapkin
Copy link
Member Author

Oh I see! I don't think we are committing to the interface until stabilization, but yes, the interface seems uncontroversial 👍🏻

@est31
Copy link
Member

est31 commented May 16, 2023

One nice project before stabilization would be to make a PR to memoffset to use offset_of internally behind a cfg or feature flag, then test that together with some users of memoffset. Maybe it could be integrated into a crater run in some fashion.

@BurntSushi
Copy link
Member

I wanted to double check that the lang team was okay with us making the call on an API like this, and indeed, they did respond on the RFC saying they were comfortable with it being in libs-api's hands: rust-lang/rfcs#3308 (comment)

@est31
Copy link
Member

est31 commented Dec 6, 2023

The current-on-nightly syntax has precedent in c's offsetof (standard link), including the way dots work for nesting, so it has some familiarity I would presume.

But this doesn't mean that we can't do offset_of!(Type.field.field) if we really wanted to.

Regarding pattern syntax, I wonder how arrays would be represented, so what would be the pattern analog of c's offsetof(S, field.field[20].10)?

@est31
Copy link
Member

est31 commented Dec 6, 2023

IDK maybe I made the wrong estimate and it's too early still for a stabilization of offset_of before the syntax discussion advances further. But my fear is that this can drag on for months and years.

@GKFX
Copy link
Contributor

GKFX commented Dec 6, 2023

I am thinking that offset_of!(Type, self.a.b.0[n]) might be a good choice - it's a single expr which the compiler already knows how to parse, with no edge cases around 0.0 etc.

To summarise, the main options that I'm aware of and my views on them are:

  • Type.a.b - not a syntax currently used elsewhere so would need custom parsing, breaks the macro follow-set restrictions if matching the type with ty.
  • C-style - current implementation but has issues with consecutive tuple accesses and whitespace not matching the current metavariables. Needs some quite awkward parsing because of that.
  • Pattern-based - significantly more verbose, no obvious method for array indexing, but is a pre-existing syntax so no parser work
  • self.a.b - slightly more verbose version of C-style. Also should need no parser work.

@the8472
Copy link
Member

the8472 commented Dec 6, 2023

You're forgetting enums

@est31
Copy link
Member

est31 commented Dec 6, 2023

@CryZe btw, patterns have come up in the syntax discussion on zulip.

Maybe it would be best to move this discussion there? Or a dedicated thread elswhere. This is a tracking issue after all :).

@RalfJung
Copy link
Member

RalfJung commented Dec 6, 2023

Regarding pattern syntax, I wonder how arrays would be represented, so what would be the pattern analog of c's offsetof(S, field.field[20].10)?

offset_of!(S.field.field[20].10). (C does not have .10 though, is this a Rust tuple field access?)

I don't think I understand what the pattern syntax would look like, could someone give some examples? offset_of!(Struct { field1, field2 }) makes no sense so surely it'd have to be a very restricted subset of patterns, and rather redundant if one has to always add the ..?

@est31
Copy link
Member

est31 commented Dec 29, 2023

surely it'd have to be a very restricted subset of patterns, and rather redundant if one has to always add the ..?

Yeah the zulip thread proposes usage of @ for that.

C does not have .10 though, is this a Rust tuple field access?

Yes, sorry I forgot. replace that .10 with .ten :).

matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Jan 4, 2024
Make offset_of field parsing use metavariable which handles any spacing

As discussed at and around comments rust-lang#106655 (comment) and rust-lang#106655 (comment), the current arguments to offset_of do not accept all the whitespace combinations: `0. 1.1.1` and `0.1.1. 1` are currently treated specially in `tests/ui/offset-of/offset-of-tuple-nested.rs`.

They also do not allow [forwarding individual fields as in](https://play.rust-lang.org/?version=nightly&mode=debug&edition=2021&gist=444cdf0ec02b99e8fd5fd8d8ecb312ca)
```rust
macro_rules! off {
    ($a:expr) => {
        offset_of!(m::S, 0. $a)
    }
}
```

This PR replaces the macro arguments with `($Container:ty, $($fields:expr)+ $(,)?)` which does allow any arrangement of whitespace that I could come up with and the forwarding of fields example above.

This also allows for array indexing in the future, which I think is the last future extension to the syntax suggested in the offset_of RFC.

Tracking issue for offset_of: rust-lang#106655
`@rustbot` label F-offset_of

`@est31`
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Jan 4, 2024
Make offset_of field parsing use metavariable which handles any spacing

As discussed at and around comments rust-lang#106655 (comment) and rust-lang#106655 (comment), the current arguments to offset_of do not accept all the whitespace combinations: `0. 1.1.1` and `0.1.1. 1` are currently treated specially in `tests/ui/offset-of/offset-of-tuple-nested.rs`.

They also do not allow [forwarding individual fields as in](https://play.rust-lang.org/?version=nightly&mode=debug&edition=2021&gist=444cdf0ec02b99e8fd5fd8d8ecb312ca)
```rust
macro_rules! off {
    ($a:expr) => {
        offset_of!(m::S, 0. $a)
    }
}
```

This PR replaces the macro arguments with `($Container:ty, $($fields:expr)+ $(,)?)` which does allow any arrangement of whitespace that I could come up with and the forwarding of fields example above.

This also allows for array indexing in the future, which I think is the last future extension to the syntax suggested in the offset_of RFC.

Tracking issue for offset_of: rust-lang#106655
``@rustbot`` label F-offset_of

``@est31``
rust-timer added a commit to rust-lang-ci/rust that referenced this issue Jan 4, 2024
Rollup merge of rust-lang#119532 - GKFX:offset-of-parse-expr, r=est31

Make offset_of field parsing use metavariable which handles any spacing

As discussed at and around comments rust-lang#106655 (comment) and rust-lang#106655 (comment), the current arguments to offset_of do not accept all the whitespace combinations: `0. 1.1.1` and `0.1.1. 1` are currently treated specially in `tests/ui/offset-of/offset-of-tuple-nested.rs`.

They also do not allow [forwarding individual fields as in](https://play.rust-lang.org/?version=nightly&mode=debug&edition=2021&gist=444cdf0ec02b99e8fd5fd8d8ecb312ca)
```rust
macro_rules! off {
    ($a:expr) => {
        offset_of!(m::S, 0. $a)
    }
}
```

This PR replaces the macro arguments with `($Container:ty, $($fields:expr)+ $(,)?)` which does allow any arrangement of whitespace that I could come up with and the forwarding of fields example above.

This also allows for array indexing in the future, which I think is the last future extension to the syntax suggested in the offset_of RFC.

Tracking issue for offset_of: rust-lang#106655
``@rustbot`` label F-offset_of

``@est31``
github-actions bot pushed a commit to rust-lang/miri that referenced this issue Jan 5, 2024
Make offset_of field parsing use metavariable which handles any spacing

As discussed at and around comments rust-lang/rust#106655 (comment) and rust-lang/rust#106655 (comment), the current arguments to offset_of do not accept all the whitespace combinations: `0. 1.1.1` and `0.1.1. 1` are currently treated specially in `tests/ui/offset-of/offset-of-tuple-nested.rs`.

They also do not allow [forwarding individual fields as in](https://play.rust-lang.org/?version=nightly&mode=debug&edition=2021&gist=444cdf0ec02b99e8fd5fd8d8ecb312ca)
```rust
macro_rules! off {
    ($a:expr) => {
        offset_of!(m::S, 0. $a)
    }
}
```

This PR replaces the macro arguments with `($Container:ty, $($fields:expr)+ $(,)?)` which does allow any arrangement of whitespace that I could come up with and the forwarding of fields example above.

This also allows for array indexing in the future, which I think is the last future extension to the syntax suggested in the offset_of RFC.

Tracking issue for offset_of: #106655
``@rustbot`` label F-offset_of

``@est31``
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Jan 20, 2024
…wesleywiser

Stabilize single-field offset_of

This PR stabilizes offset_of for a single field. There has been some further discussion at rust-lang#106655 about whether this is advisable; I'm opening the PR anyway so that the code is available.
rust-timer added a commit to rust-lang-ci/rust that referenced this issue Jan 20, 2024
Rollup merge of rust-lang#118799 - GKFX:stabilize-simple-offsetof, r=wesleywiser

Stabilize single-field offset_of

This PR stabilizes offset_of for a single field. There has been some further discussion at rust-lang#106655 about whether this is advisable; I'm opening the PR anyway so that the code is available.
@est31
Copy link
Member

est31 commented Jan 20, 2024

As #118799 has been merged, non-nested offset_of!() is now stable on the master branch and will likely be part of the 1.77.0 stable release on March 21, 2024. For details of what was stabilized, see the stabilization report.

I'm closing this tracking issue as the still unstable aspects of the offset_of macro now have different tracking issues:

Thanks everyone involved in getting offset_of specified, implemented, tested and stabilized.

@est31 est31 closed this as completed Jan 20, 2024
@rfcbot rfcbot removed proposed-final-comment-period Proposed to merge/close by relevant subteam, see T-<team> label. Will enter FCP once signed off. disposition-merge This issue / PR is in PFCP or FCP with a disposition to merge it. labels Jan 20, 2024
github-actions bot pushed a commit to rust-lang/miri that referenced this issue Jan 21, 2024
Stabilize single-field offset_of

This PR stabilizes offset_of for a single field. There has been some further discussion at rust-lang/rust#106655 about whether this is advisable; I'm opening the PR anyway so that the code is available.
lnicola pushed a commit to lnicola/rust-analyzer that referenced this issue Apr 7, 2024
Feature gate enums in offset_of

As requested at rust-lang/rust#106655 (comment), put enums in offset_of behind their own feature gate.

`@rustbot` label F-offset_of
lnicola pushed a commit to lnicola/rust-analyzer that referenced this issue Apr 7, 2024
Remove option_payload_ptr; redundant to offset_of

The `option_payload_ptr` intrinsic is no longer required as `offset_of` supports traversing enums (#114208). This PR removes it in order to dogfood offset_of (as suggested at rust-lang/rust#106655 (comment)). However, it will not build until those changes reach beta (which I think is within the next 8 days?) so I've opened it as a draft.
RalfJung pushed a commit to RalfJung/rust-analyzer that referenced this issue Apr 27, 2024
Feature gate enums in offset_of

As requested at rust-lang/rust#106655 (comment), put enums in offset_of behind their own feature gate.

`@rustbot` label F-offset_of
RalfJung pushed a commit to RalfJung/rust-analyzer that referenced this issue Apr 27, 2024
Remove option_payload_ptr; redundant to offset_of

The `option_payload_ptr` intrinsic is no longer required as `offset_of` supports traversing enums (#114208). This PR removes it in order to dogfood offset_of (as suggested at rust-lang/rust#106655 (comment)). However, it will not build until those changes reach beta (which I think is within the next 8 days?) so I've opened it as a draft.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-tracking-issue Category: An issue tracking the progress of sth. like the implementation of an RFC F-offset_of `#![feature(offset_of)]` T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests