Tracking issue for `concat_idents` #29599

Open
aturon opened this Issue Nov 5, 2015 · 67 comments

Comments

Projects
None yet
@aturon
Member

aturon commented Nov 5, 2015

Tracks stabilization for the concat_idents macro.

@retep998

This comment has been minimized.

Show comment
Hide comment
@retep998

retep998 Nov 5, 2015

Member

concat_idents is so fundamentally useless at the moment because macros cannot be used in ident positions.

Member

retep998 commented Nov 5, 2015

concat_idents is so fundamentally useless at the moment because macros cannot be used in ident positions.

@Toby-S

This comment has been minimized.

Show comment
Hide comment
@Toby-S

Toby-S Nov 5, 2015

Contributor

Yeah it is pretty much useless right now (see #12249 #13294).

Contributor

Toby-S commented Nov 5, 2015

Yeah it is pretty much useless right now (see #12249 #13294).

@nrc

This comment has been minimized.

Show comment
Hide comment
@eddyb

This comment has been minimized.

Show comment
Hide comment
@eddyb

eddyb Nov 24, 2015

Member

A solution that has been thrown around on IRC (but never ended up anywhere else AFAIK):
Have a macro invocation form for "early expansion", i.e. the following would work without macros in ident position:

macro_rules! wrap {
    ($name:ident) => { struct concat_idents!!(Wrapped, $name)($name); }
}

The "early expansion" macro_name!!(args) syntax was originally $*macro_name!(args) and is a subject of bikeshed.

If we move towards having all macros produce token streams that are parsed on-demand, such "early expansion" could be used in many more locations without adding macro support for each one.
Generic parameters, ADT fields, function signatures and match arms come to mind - there are so many recursive push-down hacks macros can end up with, just to construct lists of things and whatnot, that would simply not be necessary with "early expansion".

The only disadvantage is that concat_idents!! and friends would only work inside the RHS of macro_rules!, but I don't really see why you would need to use such a capability outside of a macro.

Member

eddyb commented Nov 24, 2015

A solution that has been thrown around on IRC (but never ended up anywhere else AFAIK):
Have a macro invocation form for "early expansion", i.e. the following would work without macros in ident position:

macro_rules! wrap {
    ($name:ident) => { struct concat_idents!!(Wrapped, $name)($name); }
}

The "early expansion" macro_name!!(args) syntax was originally $*macro_name!(args) and is a subject of bikeshed.

If we move towards having all macros produce token streams that are parsed on-demand, such "early expansion" could be used in many more locations without adding macro support for each one.
Generic parameters, ADT fields, function signatures and match arms come to mind - there are so many recursive push-down hacks macros can end up with, just to construct lists of things and whatnot, that would simply not be necessary with "early expansion".

The only disadvantage is that concat_idents!! and friends would only work inside the RHS of macro_rules!, but I don't really see why you would need to use such a capability outside of a macro.

@skade

This comment has been minimized.

Show comment
Hide comment
@skade

This comment has been minimized.

Show comment
Hide comment
@skade

skade Dec 1, 2015

Contributor

Well, given that mikkyang/rust-blas#12 is a nicer solution within the current language, I'd also like to raise my hand for "useless".

Contributor

skade commented Dec 1, 2015

Well, given that mikkyang/rust-blas#12 is a nicer solution within the current language, I'd also like to raise my hand for "useless".

@nrc

This comment has been minimized.

Show comment
Hide comment
@nrc

nrc Mar 14, 2016

Member

I think we should never stabilise the current version - as others have noted it is useless.

Member

nrc commented Mar 14, 2016

I think we should never stabilise the current version - as others have noted it is useless.

@aidanhs

This comment has been minimized.

Show comment
Hide comment
@aidanhs

aidanhs May 29, 2016

Member

Since this is the tracking issue, other who stumble across it might be interested in https://github.com/SkylerLipthay/interpolate_idents which works on nightly rust.

Member

aidanhs commented May 29, 2016

Since this is the tracking issue, other who stumble across it might be interested in https://github.com/SkylerLipthay/interpolate_idents which works on nightly rust.

@retep998

This comment has been minimized.

Show comment
Hide comment
@retep998

retep998 May 29, 2016

Member

Unfortunately that solution will always depend on nightly Rust (until the day that syntax extensions become stable and pigs fly).

Member

retep998 commented May 29, 2016

Unfortunately that solution will always depend on nightly Rust (until the day that syntax extensions become stable and pigs fly).

@skade

This comment has been minimized.

Show comment
Hide comment
@skade

skade May 30, 2016

Contributor

Given that the consensus seems to be "useless" and theres nicer solutions (e.g. [1], maybe we can remove this feature?

[1] https://github.com/mikkyang/rust-blas/pull/12/files

Contributor

skade commented May 30, 2016

Given that the consensus seems to be "useless" and theres nicer solutions (e.g. [1], maybe we can remove this feature?

[1] https://github.com/mikkyang/rust-blas/pull/12/files

@eddyb

This comment has been minimized.

Show comment
Hide comment
Member

eddyb commented May 30, 2016

@aidanhs

This comment has been minimized.

Show comment
Hide comment
@aidanhs

aidanhs May 30, 2016

Member

@retep998 yes, just noting it as a stopgap.

@skade it's the implementation rather than the idea that's useless. If concat_idents is removed rather than fixed, something else needs to fill its place (your solution doesn't work for all use-cases). I like the look of the rfc @eddyb linked, will follow that.

Member

aidanhs commented May 30, 2016

@retep998 yes, just noting it as a stopgap.

@skade it's the implementation rather than the idea that's useless. If concat_idents is removed rather than fixed, something else needs to fill its place (your solution doesn't work for all use-cases). I like the look of the rfc @eddyb linked, will follow that.

@skade

This comment has been minimized.

Show comment
Hide comment
@skade

skade May 30, 2016

Contributor

@aidanhs that still means that we should drop the feature to make sure no one uses it.

I agree that my solution doesn't cover all edge-cases, but all uses that I've currently seen in the wild. Also, but this is outside of my competence to decide, I think this is a perfect case for the use of a code generator.

Contributor

skade commented May 30, 2016

@aidanhs that still means that we should drop the feature to make sure no one uses it.

I agree that my solution doesn't cover all edge-cases, but all uses that I've currently seen in the wild. Also, but this is outside of my competence to decide, I think this is a perfect case for the use of a code generator.

@aidanhs

This comment has been minimized.

Show comment
Hide comment
@aidanhs

aidanhs Jun 1, 2016

Member

@skade afaict nobody can use it, so that's not a big concern...but equally there's no reason to keep it around. My main motivation for leaving it was as a reminder that people do want the functionality, but I'm content to follow the RFC and have changed my position to "I don't mind either way".

Regarding use cases, here's the motivating example that brought me to this issue in the first place. Codegen's awesome for things like phf, but it feels a bit like overkill when I just want to generate some repetitive methods to extract tuples from enum variants. I guess it's down to personal preference.

Member

aidanhs commented Jun 1, 2016

@skade afaict nobody can use it, so that's not a big concern...but equally there's no reason to keep it around. My main motivation for leaving it was as a reminder that people do want the functionality, but I'm content to follow the RFC and have changed my position to "I don't mind either way".

Regarding use cases, here's the motivating example that brought me to this issue in the first place. Codegen's awesome for things like phf, but it feels a bit like overkill when I just want to generate some repetitive methods to extract tuples from enum variants. I guess it's down to personal preference.

@ExpHP

This comment has been minimized.

Show comment
Hide comment
@ExpHP

ExpHP Nov 24, 2017

Contributor

An amusing fact about the current implementation of concat_idents! is that it will accept an empty argument list, making it possible to construct the empty string as an identifier:

error[E0425]: cannot find value `` in this scope
 --> src/main.rs:5:5
  |
5 |     concat_idents!();
  |     ^^^^^^^^^^^^^^^^^ not found in this scope

(good luck actually doing anything with it)

Contributor

ExpHP commented Nov 24, 2017

An amusing fact about the current implementation of concat_idents! is that it will accept an empty argument list, making it possible to construct the empty string as an identifier:

error[E0425]: cannot find value `` in this scope
 --> src/main.rs:5:5
  |
5 |     concat_idents!();
  |     ^^^^^^^^^^^^^^^^^ not found in this scope

(good luck actually doing anything with it)

@petrochenkov

This comment has been minimized.

Show comment
Hide comment
@petrochenkov

petrochenkov Nov 24, 2017

Contributor

construct the empty string as an identifier

Empty string is used as a reserved identifier with special meaning in several places in the compiler, so this can potentially cause issues.

Contributor

petrochenkov commented Nov 24, 2017

construct the empty string as an identifier

Empty string is used as a reserved identifier with special meaning in several places in the compiler, so this can potentially cause issues.

@jonhoo

This comment has been minimized.

Show comment
Hide comment
@jonhoo

jonhoo Jan 14, 2018

Contributor

The linked RFC has been closed as postponed. Should concat_idents! now be removed? Is there any chance of seeing something in its place that supports generating identifiers?

Contributor

jonhoo commented Jan 14, 2018

The linked RFC has been closed as postponed. Should concat_idents! now be removed? Is there any chance of seeing something in its place that supports generating identifiers?

@durka

This comment has been minimized.

Show comment
Hide comment
@durka

durka Jan 15, 2018

Contributor

The RFC should be reopened. It never should have been closed, as no better solution was proposed.

Contributor

durka commented Jan 15, 2018

The RFC should be reopened. It never should have been closed, as no better solution was proposed.

@skade

This comment has been minimized.

Show comment
Hide comment
@skade

skade Jan 15, 2018

Contributor

@durka I don't agree. There's ample reason to expect a new one when moving towards macros 2.0. RFCs are not necessarily closed to be replaced immediately, they are closed to stop following that line of discussion. Future RFCs might refer to it.

Contributor

skade commented Jan 15, 2018

@durka I don't agree. There's ample reason to expect a new one when moving towards macros 2.0. RFCs are not necessarily closed to be replaced immediately, they are closed to stop following that line of discussion. Future RFCs might refer to it.

@alexreg

This comment has been minimized.

Show comment
Hide comment
@alexreg

alexreg Jan 15, 2018

Contributor

I'm with @durka on this.

Contributor

alexreg commented Jan 15, 2018

I'm with @durka on this.

@eddyb

This comment has been minimized.

Show comment
Hide comment
@eddyb

eddyb Jan 15, 2018

Member

@skade I don't think much has happened regarding macros 2.0 design except for maybe some implementation experience - e.g. @jseyfried might be aware of potential issues with early expansion.

Member

eddyb commented Jan 15, 2018

@skade I don't think much has happened regarding macros 2.0 design except for maybe some implementation experience - e.g. @jseyfried might be aware of potential issues with early expansion.

@durka

This comment has been minimized.

Show comment
Hide comment
@durka

durka Jan 15, 2018

Contributor

@skade, we are moving towards macros 2.0 as far as I can tell. rust-lang/rfcs#1584 was merged (though the promised follow-ups didn't seem to come) and #![feature(decl_macro)] exists on nightly. When is the appropriate time to reopen all the concerns that were tabled as "wait for macros 2.0"?

Contributor

durka commented Jan 15, 2018

@skade, we are moving towards macros 2.0 as far as I can tell. rust-lang/rfcs#1584 was merged (though the promised follow-ups didn't seem to come) and #![feature(decl_macro)] exists on nightly. When is the appropriate time to reopen all the concerns that were tabled as "wait for macros 2.0"?

@skade

This comment has been minimized.

Show comment
Hide comment
@skade

skade Jan 15, 2018

Contributor

@eddyb @durka that is both true, my point is that this probably merits a new RFC, based on this one, except of just reopening.

Contributor

skade commented Jan 15, 2018

@eddyb @durka that is both true, my point is that this probably merits a new RFC, based on this one, except of just reopening.

@dtolnay

This comment has been minimized.

Show comment
Hide comment
@dtolnay

dtolnay Apr 28, 2018

Member

I implemented a stable concat_idents approach that works with any compiler >=1.15.0. https://github.com/dtolnay/mashup

Member

dtolnay commented Apr 28, 2018

I implemented a stable concat_idents approach that works with any compiler >=1.15.0. https://github.com/dtolnay/mashup

@alexreg

This comment has been minimized.

Show comment
Hide comment
@alexreg

alexreg Apr 28, 2018

Contributor

@dtolnay Black magic! Good job though. :-) Does Macros 1.2 support this yet? CC @jseyfried

Contributor

alexreg commented Apr 28, 2018

@dtolnay Black magic! Good job though. :-) Does Macros 1.2 support this yet? CC @jseyfried

@ExpHP

This comment has been minimized.

Show comment
Hide comment
@ExpHP

ExpHP Apr 28, 2018

Contributor

Does Macros 1.2 support this yet?

If Macros 1.2 added support for any form of early expansion, everybody would know about it. I mean, it'd make international news.

Contributor

ExpHP commented Apr 28, 2018

Does Macros 1.2 support this yet?

If Macros 1.2 added support for any form of early expansion, everybody would know about it. I mean, it'd make international news.

@alexreg

This comment has been minimized.

Show comment
Hide comment
@alexreg

alexreg Apr 28, 2018

Contributor

Hah. I know it was being discussed properly at one point... guess it never got too far!

Contributor

alexreg commented Apr 28, 2018

Hah. I know it was being discussed properly at one point... guess it never got too far!

@dtolnay dtolnay referenced this issue in rust-lang/rfcs May 2, 2018

Closed

Generate ident of macros to call #1167

@dtolnay dtolnay added the T-libs label May 2, 2018

@dtolnay

This comment has been minimized.

Show comment
Hide comment
@dtolnay

dtolnay May 2, 2018

Member

I would like to propose stabilizing concat_idents! as it currently exists. As discussed at length in this thread, concat_idents does not solve all use cases for concatenated identifiers especially as a consequence of no macro expansion in identifier position. Nevertheless, this macro does a useful thing and has its uses for putting together certain types of convenient APIs.

Basically I don't feel the need to push for macro expansion in more places before stabilizing use of concat_idents in expression position.

For more elaborate use cases of concatenated identifiers I believe directing people to use mashup is acceptable.

@rfcbot fcp merge

Member

dtolnay commented May 2, 2018

I would like to propose stabilizing concat_idents! as it currently exists. As discussed at length in this thread, concat_idents does not solve all use cases for concatenated identifiers especially as a consequence of no macro expansion in identifier position. Nevertheless, this macro does a useful thing and has its uses for putting together certain types of convenient APIs.

Basically I don't feel the need to push for macro expansion in more places before stabilizing use of concat_idents in expression position.

For more elaborate use cases of concatenated identifiers I believe directing people to use mashup is acceptable.

@rfcbot fcp merge

@eddyb

This comment has been minimized.

Show comment
Hide comment
@eddyb

eddyb May 3, 2018

Member

@joshtriplett What do you mean? This macro can't be implemented in a library, it's already built-in.
However, macro invocations are not allowed in identifier positions which simplifies many things (how do you parse a macro invocation when its name could be produced by another macro?), and this is where "early expansion" comes in, because then you're "just" expanding arbitrary tokens, not ASTs.

Member

eddyb commented May 3, 2018

@joshtriplett What do you mean? This macro can't be implemented in a library, it's already built-in.
However, macro invocations are not allowed in identifier positions which simplifies many things (how do you parse a macro invocation when its name could be produced by another macro?), and this is where "early expansion" comes in, because then you're "just" expanding arbitrary tokens, not ASTs.

@durka

This comment has been minimized.

Show comment
Hide comment
@durka

durka May 3, 2018

Contributor

@joshtriplett I don't think concat_idents is at fault, really. The issue is that macros can't be expanded in certain positions. Something would have to change around that, or around eager expansion, to be able to use concat_idents for something like constructing the names of getter/setter functions. The other issue is hygiene, but that is going to be fixed with proc macros in another way (punted for now).

Contributor

durka commented May 3, 2018

@joshtriplett I don't think concat_idents is at fault, really. The issue is that macros can't be expanded in certain positions. Something would have to change around that, or around eager expansion, to be able to use concat_idents for something like constructing the names of getter/setter functions. The other issue is hygiene, but that is going to be fixed with proc macros in another way (punted for now).

@alexreg

This comment has been minimized.

Show comment
Hide comment
@alexreg

alexreg May 3, 2018

Contributor

@eddyb Ah, I thought it was just a hygiene problem... Looks like it's a hygiene and early-expansion problem.

@durka Yep, I'm working on the hygiene part. We've got what seems like a pretty good approach now, but we're actually going to implement two and feature-gate them, then experiment with them.

Contributor

alexreg commented May 3, 2018

@eddyb Ah, I thought it was just a hygiene problem... Looks like it's a hygiene and early-expansion problem.

@durka Yep, I'm working on the hygiene part. We've got what seems like a pretty good approach now, but we're actually going to implement two and feature-gate them, then experiment with them.

@mark-i-m

This comment has been minimized.

Show comment
Hide comment
@mark-i-m

mark-i-m May 3, 2018

Contributor
Contributor

mark-i-m commented May 3, 2018

@ExpHP

This comment has been minimized.

Show comment
Hide comment
@ExpHP

ExpHP May 3, 2018

Contributor

Is there a reason why it can't be used in patterns?

error: non-pattern macro in pattern position: concat_idents
 --> src/main.rs:3:9
  |
3 |     let concat_idents!(fn) = 4;
  |         ^^^^^^^^^^^^^
Contributor

ExpHP commented May 3, 2018

Is there a reason why it can't be used in patterns?

error: non-pattern macro in pattern position: concat_idents
 --> src/main.rs:3:9
  |
3 |     let concat_idents!(fn) = 4;
  |         ^^^^^^^^^^^^^
@ExpHP

This comment has been minimized.

Show comment
Hide comment
@ExpHP

ExpHP May 3, 2018

Contributor

Oh and FYI you can construct keywords as identifiers, too =P

Edit: I'm dumb, that's probably intentional for the sake of raw identifiers

Contributor

ExpHP commented May 3, 2018

Oh and FYI you can construct keywords as identifiers, too =P

Edit: I'm dumb, that's probably intentional for the sake of raw identifiers

@cramertj

This comment has been minimized.

Show comment
Hide comment
@cramertj

cramertj May 3, 2018

Member

@rfcbot concern naming

Practically every use I've had for concat_idents has required being able to use macros in identifier position, so if I discovered a stable macro called concat_idents I'd certainly expect this to work. Can we come up with a better name for the macro that is suggestive of its limited usability, while also leaving the door open to a macro that "does what you want" such as mashup being called concat_idents!?

Member

cramertj commented May 3, 2018

@rfcbot concern naming

Practically every use I've had for concat_idents has required being able to use macros in identifier position, so if I discovered a stable macro called concat_idents I'd certainly expect this to work. Can we come up with a better name for the macro that is suggestive of its limited usability, while also leaving the door open to a macro that "does what you want" such as mashup being called concat_idents!?

@sfackler

This comment has been minimized.

Show comment
Hide comment
@sfackler

sfackler May 3, 2018

Member

The macro does what it says on the tin - it concatenates idents. The changes required to make it work in positions it doesn't yet aren't in the macro, they're in the rest of the compiler.

Member

sfackler commented May 3, 2018

The macro does what it says on the tin - it concatenates idents. The changes required to make it work in positions it doesn't yet aren't in the macro, they're in the rest of the compiler.

@alexreg

This comment has been minimized.

Show comment
Hide comment
@alexreg

alexreg May 3, 2018

Contributor

@cramertj mashup is a big hack, right now, as I see. Anyway, it's a 3rd-party crate, and I don't see why we should be concerned about official Rust identifiers clashing with 3rd-party ones. Once eager-expansion and hygiene land, it will do what we all want.

Contributor

alexreg commented May 3, 2018

@cramertj mashup is a big hack, right now, as I see. Anyway, it's a 3rd-party crate, and I don't see why we should be concerned about official Rust identifiers clashing with 3rd-party ones. Once eager-expansion and hygiene land, it will do what we all want.

@ExpHP

This comment has been minimized.

Show comment
Hide comment
@ExpHP

ExpHP May 3, 2018

Contributor

@sfackler if that was directed to me, pattern macros already exist.

If not, uh... carry on!

Contributor

ExpHP commented May 3, 2018

@sfackler if that was directed to me, pattern macros already exist.

If not, uh... carry on!

@durka

This comment has been minimized.

Show comment
Hide comment
@durka

durka May 3, 2018

Contributor
Contributor

durka commented May 3, 2018

@durka

This comment has been minimized.

Show comment
Hide comment
@durka

durka May 3, 2018

Contributor
Contributor

durka commented May 3, 2018

@retep998

This comment has been minimized.

Show comment
Hide comment
@retep998

retep998 May 3, 2018

Member

I'd love to see someone do a demonstration where they adjust winapi's macros to use mashup and eliminate the redundant inputs I have to specify due to the lack of a useful concat_idents.

Member

retep998 commented May 3, 2018

I'd love to see someone do a demonstration where they adjust winapi's macros to use mashup and eliminate the redundant inputs I have to specify due to the lack of a useful concat_idents.

@joshtriplett

This comment has been minimized.

Show comment
Hide comment
@joshtriplett

joshtriplett May 3, 2018

Member

@eddyb I can understand not being able to produce a macro invocation with a macro, for instance. But why does that prevent, for instance, generating the name of a function or variable?

Member

joshtriplett commented May 3, 2018

@eddyb I can understand not being able to produce a macro invocation with a macro, for instance. But why does that prevent, for instance, generating the name of a function or variable?

@dtolnay

This comment has been minimized.

Show comment
Hide comment
@dtolnay

dtolnay May 3, 2018

Member

Sorry about the search results @durka, obviously 💔 GitHub search. I clicked through the first 8 pages and found the following diverse and wonderful use cases. These crates are using concat_idents because it does exactly what is needed for their situation. I am happy to dig through all 27 pages if this does not change your mind about the macro being "tantalizing but useless and should be deprecated."

I believe concat_idents! is a valuable building block for macros, much like stringify!, despite not solving everybody's use cases all the time.


Member

dtolnay commented May 3, 2018

Sorry about the search results @durka, obviously 💔 GitHub search. I clicked through the first 8 pages and found the following diverse and wonderful use cases. These crates are using concat_idents because it does exactly what is needed for their situation. I am happy to dig through all 27 pages if this does not change your mind about the macro being "tantalizing but useless and should be deprecated."

I believe concat_idents! is a valuable building block for macros, much like stringify!, despite not solving everybody's use cases all the time.


@dtolnay

This comment has been minimized.

Show comment
Hide comment
@dtolnay

dtolnay May 3, 2018

Member

Through page 13:


Member

dtolnay commented May 3, 2018

Through page 13:


@dtolnay

This comment has been minimized.

Show comment
Hide comment
@dtolnay

dtolnay May 3, 2018

Member
Member

dtolnay commented May 3, 2018

@petrochenkov

This comment has been minimized.

Show comment
Hide comment
@petrochenkov

petrochenkov May 3, 2018

Contributor

@rfcbot concern: Audit hygienic context of the produced identifier, add relevant tests

Context of the ident should probably be the context of concat_idents! macro invocation (#50122), aka Span::call_site().

Contributor

petrochenkov commented May 3, 2018

@rfcbot concern: Audit hygienic context of the produced identifier, add relevant tests

Context of the ident should probably be the context of concat_idents! macro invocation (#50122), aka Span::call_site().

@alexreg

This comment has been minimized.

Show comment
Hide comment
@alexreg

alexreg May 3, 2018

Contributor

@durka Hygiene opt-out/escaping landing, I mean.

Contributor

alexreg commented May 3, 2018

@durka Hygiene opt-out/escaping landing, I mean.

@durka

This comment has been minimized.

Show comment
Hide comment
@durka

durka May 3, 2018

Contributor
Contributor

durka commented May 3, 2018

@alexreg

This comment has been minimized.

Show comment
Hide comment
@alexreg

alexreg May 3, 2018

Contributor

@durka Yeah. I feel concat_idents + early expansion + hygiene escaping is the more elegant and general solution that fits all potential use cases.

Contributor

alexreg commented May 3, 2018

@durka Yeah. I feel concat_idents + early expansion + hygiene escaping is the more elegant and general solution that fits all potential use cases.

@dtolnay

This comment has been minimized.

Show comment
Hide comment
@dtolnay

dtolnay May 3, 2018

Member

@petrochenkov currently concat_idents cannot refer to a captured local variable, only an item and items do not have hygiene. Do you think concat_idents is different enough from other cases of non-atomic code fragments that we need to block stabilization until the non-atomic hygiene story is worked out? I would think that can happen later and does not need to be a blocker.

Member

dtolnay commented May 3, 2018

@petrochenkov currently concat_idents cannot refer to a captured local variable, only an item and items do not have hygiene. Do you think concat_idents is different enough from other cases of non-atomic code fragments that we need to block stabilization until the non-atomic hygiene story is worked out? I would think that can happen later and does not need to be a blocker.

@petrochenkov

This comment has been minimized.

Show comment
Hide comment
@petrochenkov

petrochenkov May 3, 2018

Contributor

@dtolnay

currently concat_idents cannot refer to a captured local variable

That's a bug.

and items do not have hygiene.

All identifiers have hygiene with macros >= 1.2!
concat_idents looks like a typical use case for Span::call_site hygiene.

Do you think concat_idents is different enough from other cases of non-atomic code fragments that we need to block stabilization until the non-atomic hygiene story is worked out?

concat_idents is not special, #50122 (or rather its minimal subset about call-site for macro invocations) is a blocker for it the same degree as for Macros 1.2 using Span::call_site in general.

Contributor

petrochenkov commented May 3, 2018

@dtolnay

currently concat_idents cannot refer to a captured local variable

That's a bug.

and items do not have hygiene.

All identifiers have hygiene with macros >= 1.2!
concat_idents looks like a typical use case for Span::call_site hygiene.

Do you think concat_idents is different enough from other cases of non-atomic code fragments that we need to block stabilization until the non-atomic hygiene story is worked out?

concat_idents is not special, #50122 (or rather its minimal subset about call-site for macro invocations) is a blocker for it the same degree as for Macros 1.2 using Span::call_site in general.

@durka

This comment has been minimized.

Show comment
Hide comment
@durka

durka May 3, 2018

Contributor
Contributor

durka commented May 3, 2018

@petrochenkov

This comment has been minimized.

Show comment
Hide comment
@petrochenkov

petrochenkov May 3, 2018

Contributor

@durka
I'd expect contexts of components to be completely lost during concatenation, it's a purely string-content based operation after all.

Contributor

petrochenkov commented May 3, 2018

@durka
I'd expect contexts of components to be completely lost during concatenation, it's a purely string-content based operation after all.

@alexreg

This comment has been minimized.

Show comment
Hide comment
@alexreg

alexreg May 3, 2018

Contributor

@durka I'd expect the same as @petrochenkov. The identifier would then take the syntax context of its call site (i.e. where it's used in code), unless escaped (e.g. using # syntax). We have discussed the #[escapes(...)] approach to escaping hygiene as well, but unless we allow macro expansion within escapes(...), this becomes a real problem.

Contributor

alexreg commented May 3, 2018

@durka I'd expect the same as @petrochenkov. The identifier would then take the syntax context of its call site (i.e. where it's used in code), unless escaped (e.g. using # syntax). We have discussed the #[escapes(...)] approach to escaping hygiene as well, but unless we allow macro expansion within escapes(...), this becomes a real problem.

@durka

This comment has been minimized.

Show comment
Hide comment
@durka

durka May 3, 2018

Contributor
Contributor

durka commented May 3, 2018

@nrc

This comment has been minimized.

Show comment
Hide comment
@nrc

nrc May 3, 2018

Member

@concern premature

I don't think we should stabilise concat_idents, between the macro system and the macro itself, this is not the macro that most macro authors want, but it is occupying prime naming real estate. I would prefer to wait until we have a macro that does do what people want before stabilising something.

Is there a strong reason to stabilise this now when we've been OK without stabilising it for years? It does seem like a really sub-optimal thing to stabilise.

Member

nrc commented May 3, 2018

@concern premature

I don't think we should stabilise concat_idents, between the macro system and the macro itself, this is not the macro that most macro authors want, but it is occupying prime naming real estate. I would prefer to wait until we have a macro that does do what people want before stabilising something.

Is there a strong reason to stabilise this now when we've been OK without stabilising it for years? It does seem like a really sub-optimal thing to stabilise.

@alexreg

This comment has been minimized.

Show comment
Hide comment
@alexreg

alexreg May 6, 2018

Contributor

@nrc It does what it says on the tin though, as has been pointed out. Shouldn't the aim just be to work on early (eager) expansion now?

Contributor

alexreg commented May 6, 2018

@nrc It does what it says on the tin though, as has been pointed out. Shouldn't the aim just be to work on early (eager) expansion now?

@durka

This comment has been minimized.

Show comment
Hide comment
@durka

durka May 6, 2018

Contributor
Contributor

durka commented May 6, 2018

@alexreg

This comment has been minimized.

Show comment
Hide comment
@alexreg

alexreg May 6, 2018

Contributor
  1. or 3. sound best right now.
Contributor

alexreg commented May 6, 2018

  1. or 3. sound best right now.
@dtolnay

This comment has been minimized.

Show comment
Hide comment
@dtolnay

dtolnay May 8, 2018

Member

I continue to believe this macro in its current form serves as a valuable building block for API design despite being limited to solving a specific set of use cases and not them all. There comes a time in an experienced macro author's life when they have seen many dozen situations that require exactly this functionality and wonder what the holdup could be after almost 7 years. But I can see that changing so many minds is not going to be a good use of two teams' time. Thanks for the discussion everyone! 🙂

@rfcbot fcp cancel

Member

dtolnay commented May 8, 2018

I continue to believe this macro in its current form serves as a valuable building block for API design despite being limited to solving a specific set of use cases and not them all. There comes a time in an experienced macro author's life when they have seen many dozen situations that require exactly this functionality and wonder what the holdup could be after almost 7 years. But I can see that changing so many minds is not going to be a good use of two teams' time. Thanks for the discussion everyone! 🙂

@rfcbot fcp cancel

@rfcbot

This comment has been minimized.

Show comment
Hide comment
@rfcbot

rfcbot May 8, 2018

@dtolnay proposal cancelled.

rfcbot commented May 8, 2018

@dtolnay proposal cancelled.

@alexreg

This comment has been minimized.

Show comment
Hide comment
@alexreg

alexreg May 8, 2018

Contributor

@dtolnay Sounds fair. Maybe we can visit this when hygiene escape support and eager expansion land, both of which I'm currently working on.

Contributor

alexreg commented May 8, 2018

@dtolnay Sounds fair. Maybe we can visit this when hygiene escape support and eager expansion land, both of which I'm currently working on.

@fafhrd91 fafhrd91 referenced this issue in PyO3/pyo3 May 12, 2018

Open

Compile with stable rust #5

2 of 5 tasks complete
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment