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 nested field access in offset_of #120140

Closed
3 tasks done
GKFX opened this issue Jan 19, 2024 · 11 comments · Fixed by #128284
Closed
3 tasks done

Tracking Issue for nested field access in offset_of #120140

GKFX opened this issue Jan 19, 2024 · 11 comments · Fixed by #128284
Labels
B-unstable Blocker: Implemented in the nightly compiler and unstable. C-tracking-issue Category: A tracking issue for an RFC or an unstable feature. disposition-merge This issue / PR is in PFCP or FCP with a disposition to merge it. F-offset_of_nested `#![feature(offset_of_nested)]` finished-final-comment-period The final comment period is finished for this PR / Issue. T-lang Relevant to the language team, which will review and decide on the PR/issue. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.

Comments

@GKFX
Copy link
Contributor

GKFX commented Jan 19, 2024

Feature gate: #![feature(offset_of_nested)]

This is a tracking issue for the use of nested field access in offset_of. This is currently implemented with dot-separated fields, like in C. The original RFC for offset_of was rust-lang/rfcs#3308.

Public API

pub macro offset_of($Container:ty, $($fields:expr)+ $(,)?) { ... }

struct Struct {
    field: A,
}
struct A {
    a: ((u8, u8), bool),
}
const OFFSET: usize = offset_of!(Struct, field.a.0.1);

Steps / History

Unresolved Questions

  • None yet.

Footnotes

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

@GKFX GKFX added C-tracking-issue Category: A tracking issue for an RFC or an unstable feature. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. labels Jan 19, 2024
@est31
Copy link
Member

est31 commented Jan 20, 2024

This has been split off from #106655 to get the non-nested offset_off macro stabilized sooner.

How the syntax for nested access should look like is still subject to discussion, especially also how it relates to enums.

Some links:

@tgross35
Copy link
Contributor

Could the top example be updated to show what Struct looks like?

@GKFX
Copy link
Contributor Author

GKFX commented Jun 2, 2024

How close is this to stabilization? It looks like this is in use by the Linux kernel (https://lore.kernel.org/rust-for-linux/CAH5fLgjP98pS1wsP=YXP5Yr79Y62VF7XPKjbj+G75B3SOFt80g@mail.gmail.com/), as well as the standard library (for an enum):

(self as *const Self).byte_add(core::mem::offset_of!(Self, Some.0)).cast(),

Stabilizing this may restrict future changes to the offset_of parsing logic (compiler/rustc_parse/src/parser/expr.rs:1191, parse_floating_field_access). Currently this has enum variants treated the same as fields and (with parsing support only) array indexing using [expr] syntax.

@fmease fmease added the B-unstable Blocker: Implemented in the nightly compiler and unstable. label Jun 9, 2024
@dingxiangfei2009
Copy link
Contributor

Cross-posting stabilization report and request because the two feature gates need to stabilize together for best utility.

Report

cc @Darksonn @ojeda

@Amanieu Amanieu added the T-lang Relevant to the language team, which will review and decide on the PR/issue. label Jun 17, 2024
@Amanieu
Copy link
Member

Amanieu commented Jun 17, 2024

@rfcbot fcp merge

@rfcbot
Copy link

rfcbot commented Jun 17, 2024

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

No concerns currently listed.

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!

cc @rust-lang/lang-advisors: FCP proposed for lang, please feel free to register concerns.
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 Jun 17, 2024
@tmandry
Copy link
Member

tmandry commented Jun 20, 2024

I'm unclear on whether this needs an RFC. I would defer to libs-api on that question though, since this is more a question of the syntax the macro supports than the capability it provides, and I think the syntax is more in the libs-api realm than the lang one (though it may be a bit of both).

Regarding the syntax, there is still an open question about supporting enum variants in the future. I'm personally okay with moving forward with this subset for now, especially since I can imagine a syntax I don't dislike: offset_of!(Struct, (field.a as MyEnum::Variant).0.1). But I'm just documenting my thinking here for future reference; we should leave the bikeshed out of this issue if at all possible.

@rfcbot reviewed

@nikomatsakis
Copy link
Contributor

@rfcbot reviewed

I don't feel this requires an RFC. I think it is a natural extension to the existing functionality.

@rfcbot rfcbot added the final-comment-period In the final comment period and will be merged soon unless new substantive objections are raised. label Jul 18, 2024
@rfcbot
Copy link

rfcbot commented Jul 18, 2024

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

@rfcbot rfcbot removed the proposed-final-comment-period Proposed to merge/close by relevant subteam, see T-<team> label. Will enter FCP once signed off. label Jul 18, 2024
@pnkfelix
Copy link
Member

@rfcbot reviewed

@rfcbot rfcbot added finished-final-comment-period The final comment period is finished for this PR / Issue. to-announce Announce this issue on triage meeting and removed final-comment-period In the final comment period and will be merged soon unless new substantive objections are raised. labels Jul 28, 2024
@rfcbot
Copy link

rfcbot commented Jul 28, 2024

The final comment period, with a disposition to merge, as per the review above, is now complete.

As the automated representative of the governance process, I would like to thank the author for their work and everyone else who contributed.

This will be merged soon.

@bors bors closed this as completed in a73a025 Jul 30, 2024
rust-timer added a commit to rust-lang-ci/rust that referenced this issue Jul 30, 2024
Rollup merge of rust-lang#128284 - GKFX:stabilize-offset-of-nested, r=dtolnay,jieyouxu

Stabilize offset_of_nested

Tracking issue rust-lang#120140. Closes rust-lang#120140.

As the FCP is now nearing its end I have opened a stabilization PR. I have done this separately to the offset_of_enum feature, since that FCP has not started.

`@rustbot` label  F-offset_of_nested T-lang T-libs-api
@apiraino apiraino removed the to-announce Announce this issue on triage meeting label Aug 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
B-unstable Blocker: Implemented in the nightly compiler and unstable. C-tracking-issue Category: A tracking issue for an RFC or an unstable feature. disposition-merge This issue / PR is in PFCP or FCP with a disposition to merge it. F-offset_of_nested `#![feature(offset_of_nested)]` finished-final-comment-period The final comment period is finished for this PR / Issue. T-lang Relevant to the language team, which will review and decide on the PR/issue. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging a pull request may close this issue.