Deprecate the unstable concat_idents!

[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

[tgross35]

r? libs-api

[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]

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:

• @Amanieu
• @BurntSushi
• @dtolnay
• @joshtriplett
• @m-ou-se
• @nikomatsakis
• @scottmcm
• @tmandry
• @traviscross

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]

@rfcbot reviewed

[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:

• Decide what we want about macro_metavar_expr #137581

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

[rfcbot]

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

[rfcbot]

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.

[JarredAllen]

Is there a plan for if/when this feature will go from deprecated to removed?

[tbu-]

Is there a plan for if/when this feature will go from deprecated to removed?

From earlier in this thread:

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

[JarredAllen]

Is there a plan for if/when this feature will go from deprecated to removed?

From earlier in this thread:

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

I wasn't sure if that "removing in the next version" was part of the approved FCP or just the PR contents itself. But if the FCP also included the removing, then that's cool with me

[tgross35]

Rebased, FCP completed so this just needs a review.

@rustbot ready

[tgross35]

Since fcp is complete I'll roll for libs

r? libs

[workingjubilee]

Cool, thanks!

@bors r+ rollup

[bors]

📌 Commit fde9fcb has been approved by workingjubilee

It is now in the queue for this repository.

[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]

@bors r=workingjubilee

[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:

rust/src/tools/replace-version-placeholder/src/main.rs

Line 24 in 3c877f6

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!