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.

Sign up for GitHub

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

Jump to bottom

[RFC-3086] Add a new concat metavar expr #111930

Closed
wants to merge 1 commit into from
Closed

[RFC-3086] Add a new concat metavar expr #111930

wants to merge 1 commit into from

Conversation

c410-f3r
Copy link
Contributor

@c410-f3r c410-f3r commented May 25, 2023 •
edited
Loading

While investigating the blockers for the stabilization of #83527, I figured it would probably be nice to have a way to concatenate identifiers as well as literals without having to use third-parties or concat_idents!.

macro_rules! create_things {
    ( $lhs:ident ) => {
        struct ${concat(lhs, _separated_idents_in_a_struct)} {
            foo: i32,
            ${concat(lhs, _separated_idents_in_a_field)}: i32,
        }

        mod ${concat(lhs, _separated_idents_in_a_module)} {
            pub const FOO: () = ();
        }

        fn ${concat(lhs, _separated_idents_in_a_fn)}() {}
    };
}

create_things!(look_ma);

fn main() {
    look_ma_separated_idents_in_a_fn();

    let _ = look_ma_separated_idents_in_a_module::FOO;

    let _ = look_ma_separated_idents_in_a_struct {
        foo: 1,
        look_ma_separated_idents_in_a_field: 2,
    };
}

The intention here is to allow further experimentation of a nightly feature to gather more feedback. If such thing is not desired, needs a RFC or anything else, feel free to close this PR.

Implementation-wise, there are some things that still need to be tweaked.

@rustbot
Copy link
Collaborator

rustbot commented May 25, 2023

r? @oli-obk

(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. labels May 25, 2023
@c410-f3r c410-f3r marked this pull request as draft May 25, 2023 03:13
@c410-f3r
Copy link
Contributor Author

cc @petrochenkov in case you want to add something

@dvdsk dvdsk mentioned this pull request Jun 1, 2023
Open
@c410-f3r
Copy link
Contributor Author

Well, no-one seemed interested so I will close this PR. At least I tried 🤷

@c410-f3r c410-f3r closed this Jun 18, 2023
@fmease
Copy link
Member

fmease commented Jun 18, 2023

Well, no-one seemed interested

I think there's a chance that you can bring this new meta-var expr to the attention of more people by opening a new Zulip topic in the T-lang stream for example.

@c410-f3r
Copy link
Contributor Author

Hum... Some of my previous attempts trying to introduce new features resulted in a simple "no" or a feedback delay of 1 year, which doesn't give me the motivation to continue pursuing this PR.

In order to get more changes of success, the overall plan is to write a RFC addressing the remaining concerns of #83527 as well as including any other useful meta variable expression (including concat).

@oli-obk
Copy link
Contributor

oli-obk commented Jun 19, 2023 •
edited
Loading

I don't know who's in charge of this feature, but I don't know enough about it to review this. I didn't even know there was an RFC and that it was accepted. I should still not have left the PR in limbo, I apologize for that.

Looking through the tracking issue, there seems to be no one really in charge of getting the feature implemented. While I can review the implementation, I do not want to make decisions on the language side of a feature I don't really know and don't have the time to get to know.

Considering that (from the tracking issue discussion), you are the expert on this feature, you can just make feature decisions and we'll get it reviewed from the impl side. Worst case this will make someone from the compiler team disagree with the changes enough to step up 😆

@c410-f3r
Copy link
Contributor Author

I don't know who's in charge of this feature, but I don't know enough about it to review this. I didn't even know there was an RFC and that it was accepted. I should still not have left the PR in limbo, I apologize for that.

Oh, no need to apologize! My concerns were directed towards the big picture and such HR situation is common among open source projects. More like a feature than a bug :)

Considering that (from the tracking issue discussion), you are the expert on this feature, you can just make feature decisions and we'll get it reviewed from the impl side. Worst case this will make someone from the compiler team disagree with the changes enough to step up laughing

It is much easier and faster to write a PR describing the intention behind the code than elaborating a formal RFC but I am still not sure because different persons showed different opinions in the tracking issue. Let me collect more material to figure things out.

@fmease
Copy link
Member

fmease commented Jun 19, 2023

In any case, I just want to say thanks for working on this! :)

@c410-f3r c410-f3r mentioned this pull request Dec 15, 2023
Merged
bors added a commit to rust-lang-ci/rust that referenced this pull request Jun 14, 2024
@bors
Auto merge of rust-lang#118958 - c410-f3r:concat-again, r=petrochenkov
f8e5660 
Add a new concat metavar expr

Revival of rust-lang#111930

Giving it another try now that rust-lang#117050 was merged.

With the new rules, meta-variable expressions must be referenced with a dollar sign (`$`) and this can cause misunderstands with `$concat`.

```rust
macro_rules! foo {
    ( $bar:ident ) => {
        const ${concat(VAR, bar)}: i32 = 1;
    };
}

// Will produce `VARbar` instead of `VAR_123`
foo!(_123);
```

In other words, forgetting the dollar symbol can produce undesired outputs.

cc rust-lang#29599
cc rust-lang#124225
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@oli-obk oli-obk

Labels
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.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@c410-f3r @rustbot @fmease @oli-obk