Skip to content

Conversation

GKFX
Copy link
Contributor

@GKFX GKFX commented Jan 2, 2024

As discussed at and around comments #106655 (comment) and #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

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

@rustbot
Copy link
Collaborator

rustbot commented Jan 2, 2024

r? @compiler-errors

(rustbot has picked a reviewer for you, use r? to override)

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-libs Relevant to the library team, which will review and decide on the PR/issue. F-offset_of `#![feature(offset_of)]` labels Jan 2, 2024
@rust-log-analyzer

This comment has been minimized.

Copy link
Member

@est31 est31 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good job! You figured out how to get parse_expr to work. I was a bit scared to use it but I suppose with that, the issues are fixed.

Some very minor nits, otherwise this looks great already.

e.emit();
}

// Eat tokens until the macro call ends.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
// Eat tokens until the macro call ends.
// Eat tokens until the builtin ends. We already error above for any unrecognized stuff.

I wonder if the check should also do the open delim/close delim matching game, for stuff like builtin # offset_of(Struct, field, ()). But probably not needed, given this is for error recovery.

@est31
Copy link
Member

est31 commented Jan 4, 2024

@bors r+

@bors
Copy link
Collaborator

bors commented Jan 4, 2024

📌 Commit f0c0a49 has been approved by est31

It is now in the queue for this repository.

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jan 4, 2024
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request 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`
bors added a commit to rust-lang-ci/rust that referenced this pull request Jan 4, 2024
…iaskrgr

Rollup of 10 pull requests

Successful merges:

 - rust-lang#117636 (add test for rust-lang#117626)
 - rust-lang#118704 (Promote `riscv32{im|imafc}` targets to tier 2)
 - rust-lang#119184 (Switch from using `//~ERROR` annotations with `--error-format` to `error-pattern`)
 - rust-lang#119325 (custom mir: make it clear what the return block is)
 - rust-lang#119391 (Use Result::flatten in catch_with_exit_code)
 - rust-lang#119431 (Support reg_addr register class in s390x inline assembly)
 - rust-lang#119475 (Remove libtest's dylib)
 - rust-lang#119532 (Make offset_of field parsing use metavariable which handles any spacing)
 - rust-lang#119553 (stop feed vis when cant access for trait item)
 - rust-lang#119556 (Reland optimized-compiler-builtins config)

r? `@ghost`
`@rustbot` modify labels: rollup
bors added a commit to rust-lang-ci/rust that referenced this pull request Jan 4, 2024
…iaskrgr

Rollup of 10 pull requests

Successful merges:

 - rust-lang#117636 (add test for rust-lang#117626)
 - rust-lang#118704 (Promote `riscv32{im|imafc}` targets to tier 2)
 - rust-lang#119184 (Switch from using `//~ERROR` annotations with `--error-format` to `error-pattern`)
 - rust-lang#119325 (custom mir: make it clear what the return block is)
 - rust-lang#119391 (Use Result::flatten in catch_with_exit_code)
 - rust-lang#119431 (Support reg_addr register class in s390x inline assembly)
 - rust-lang#119475 (Remove libtest's dylib)
 - rust-lang#119532 (Make offset_of field parsing use metavariable which handles any spacing)
 - rust-lang#119553 (stop feed vis when cant access for trait item)
 - rust-lang#119574 (Miri subtree update)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit e306cfb into rust-lang:master Jan 4, 2024
@rustbot rustbot added this to the 1.77.0 milestone Jan 4, 2024
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request 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``
@GKFX GKFX deleted the offset-of-parse-expr branch January 4, 2024 21:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
F-offset_of `#![feature(offset_of)]` S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-libs Relevant to the library team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants