Deprecate the unstable concat_idents! by tgross35 · Pull Request #137653 · rust-lang/rust (original) (raw)

Conversation

This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.Learn more about bidirectional Unicode characters

[ Show hidden characters]({{ revealButtonHref }})

tgross35

concat_idents has been around unstably for a long time, but there is now a better (but still unstable) way to join identifiers using ${concat(...)} syntax with macro_metavar_expr_concat. This resolves a lot of the problems with concat_idents and is on a better track toward stabilization, so there is no need to keep both versions around. concat_idents! still has a lot of use in the ecosystem so deprecate it before removing, as discussed in 1.

Link: #124225

@rustbot

r? @nnethercote

rustbot has assigned @nnethercote.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

@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.

labels

Feb 26, 2025

@tgross35 tgross35 changed the titleMark concat_idents! deprecated Deprecate concat_idents!

Feb 26, 2025

@tgross35

@rustbot rustbot added the T-libs-api

Relevant to the library API team, which will review and decide on the PR/issue.

label

Feb 26, 2025

@tgross35 tgross35 changed the titleDeprecate concat_idents! Deprecate the unstable concat_idents!

Feb 26, 2025

@tgross35

I'd like to propose deprecating it now (1.87) and removing in the next version (1.88), nominating for libs-api. Cc @rust-lang/lang since this is tightly coupled.

@traviscross traviscross added T-lang

Relevant to the language team, which will review and decide on the PR/issue.

I-lang-nominated

Nominated for discussion during a lang team meeting.

and removed T-libs

Relevant to the library team, which will review and decide on the PR/issue.

T-compiler

Relevant to the compiler team, which will review and decide on the PR/issue.

labels

Mar 6, 2025

@traviscross

Let's just FCP it. That's maybe weird for an unstable thing, but no more weird than doing a deprecation cycle for it.

@rfcbot fcp merge

@rfcbot

Team member @traviscross 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.

@nikomatsakis

@joshtriplett

👍 for deprecating concat_idents!, in favor of concat from macro metavar expressions.

I saw that @tgross35 proposed rust-lang/rfcs#3649 to supersede the count and len mechanism. I want to make sure that RFC 3649 is only being proposed as an alternative to count and len, and that concat is still on track to continue.

@tgross35

I saw that @tgross35 proposed rust-lang/rfcs#3649 to supersede the count and len mechanism. I want to make sure that RFC 3649 is only being proposed as an alternative to count and len, and that concat is still on track to continue.

That's correct; I think concat can move forward at any point, RFC 3649 just handles the group- and index-related things 👍

@traviscross

@rustbot labels -I-lang-nominated

We discussed this in the lang call today. Thanks @tgross35 for pushing this forward.

As above, the one question that came up was whether this was related to our open question:

But it's not, as macro_metavar_expr_concat is under a separate feature flag and isn't part of that proposed stabilization.

@rfcbot rfcbot added the final-comment-period

In the final comment period and will be merged soon unless new substantive objections are raised.

label

Mar 13, 2025

@rfcbot

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

@tgross35

Rebased, FCP completed so this just needs a review.

@rustbot ready

@tgross35

Since fcp is complete I'll roll for libs

r? libs

@rustbot rustbot added the T-libs

Relevant to the library team, which will review and decide on the PR/issue.

label

Apr 24, 2025

@workingjubilee

Cool, thanks!

@bors r+ rollup

@bors

📌 Commit fde9fcb has been approved by workingjubilee

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

Apr 24, 2025

@cuviper

I'd like to propose deprecating it now (1.87) and removing in the next version (1.88)

Note that this has slipped to 1.88, and I suppose removal in 1.89, unless we also beta-backport the deprecation.

@tgross35

Ah good catch, let me update that. It would be nice if deprecated supported RUSTC_CURRENT_VERSION

@tgross35

concat_idents has been around unstably for a long time, but there is now a better (but still unstable) way to join identifiers using ${concat(...)} syntax with macro_metavar_expr_concat. This resolves a lot of the problems with concat_idents and is on a better track toward stabilization, so there is no need to keep both versions around. concat_idents! still has a lot of use in the ecosystem so deprecate it before removing, as discussed in 1.

Link: rust-lang#124225 1: https://rust-lang.zulipchat.com/#narrow/channel/219381-t-libs/topic/Removing.20.60concat_idents.60

@tgross35

@bors

📌 Commit 75a9be6 has been approved by workingjubilee

It is now in the queue for this repository.

@cuviper

It would be nice if deprecated supported RUSTC_CURRENT_VERSION

Hmm, I think that should work -- the tool simply calls str::replace:

let new_contents = contents.replace(VERSION_PLACEHOLDER, version_str);

(although the placeholder is CURRENT_RUSTC_VERSION)

@tgross35

I did try it before and IIRC it got rejected somewhere for not being a valid version. But maybe I just made the same typo at that time too :)

@cuviper

Oh, I didn't think about attribute validation -- you're right, it doesn't work, and it should!

bors added a commit to rust-lang-ci/rust that referenced this pull request

Apr 25, 2025

@bors

…iaskrgr

Rollup of 8 pull requests

Successful merges:

r? @ghost @rustbot modify labels: rollup

rust-timer added a commit to rust-lang-ci/rust that referenced this pull request

Apr 25, 2025

@rust-timer

Rollup merge of rust-lang#137653 - tgross35:deprecate-concat_idents, r=workingjubilee

Deprecate the unstable concat_idents!

concat_idents has been around unstably for a long time, but there is now a better (but still unstable) way to join identifiers using ${concat(...)} syntax with macro_metavar_expr_concat. This resolves a lot of the problems with concat_idents and is on a better track toward stabilization, so there is no need to keep both versions around. concat_idents! still has a lot of use in the ecosystem so deprecate it before removing, as discussed in 1.

Link: rust-lang#124225

@tgross35 tgross35 deleted the deprecate-concat_idents branch

April 25, 2025 15:53

github-actions bot pushed a commit to model-checking/verify-rust-std that referenced this pull request

May 9, 2025

@matthiaskrgr

…r=workingjubilee

Deprecate the unstable concat_idents!

concat_idents has been around unstably for a long time, but there is now a better (but still unstable) way to join identifiers using ${concat(...)} syntax with macro_metavar_expr_concat. This resolves a lot of the problems with concat_idents and is on a better track toward stabilization, so there is no need to keep both versions around. concat_idents! still has a lot of use in the ecosystem so deprecate it before removing, as discussed in 1.

Link: rust-lang#124225

github-actions bot pushed a commit to model-checking/verify-rust-std that referenced this pull request

May 9, 2025

@bors

…iaskrgr

Rollup of 8 pull requests

Successful merges:

r? @ghost @rustbot modify labels: rollup

Labels

disposition-merge

This issue / PR is in PFCP or FCP with a disposition to merge it.

finished-final-comment-period

The final comment period is finished for this PR / Issue.

I-lang-easy-decision

Issue: The decision needed by the team is conjectured to be easy; this does not imply nomination

S-waiting-on-bors

Status: Waiting on bors to run and complete tests. Bors will change the label on completion.

T-lang

Relevant to the language 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.

T-libs-api

Relevant to the library API team, which will review and decide on the PR/issue.

to-announce

Announce this issue on triage meeting