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.

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

concat_idents can create empty identifiers #50403

Closed
ExpHP opened this issue May 3, 2018 · 2 comments
Closed

concat_idents can create empty identifiers #50403

ExpHP opened this issue May 3, 2018 · 2 comments

Comments

@ExpHP
Copy link
Contributor

ExpHP commented May 3, 2018

I posted this casually on the tracking issue some months back:

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)

I left it at that because I never thought there would be any serious discussion of stabilizing the feature. Never say never, huh?

This is an easy fix and I am working on a PR. I just made this issue so that I have a number to name the test case after. 🦆

@ExpHP
Copy link
Contributor Author

ExpHP commented May 3, 2018

Random aside: I thought I would try to do the same thing with proc_macro::Term. Apparently, there is a sanity check, at least in some places:

error: expected identifier, found reserved identifier ``
 --> test.rs:6:1
  |
6 | prox_max::to_the_max!{}
  | ^^^^^^^^^^^^^^^^^^^^^^^ expected identifier, found reserved identifier

error: expected expression, found reserved identifier ``
 --> test.rs:6:1
  |
6 | prox_max::to_the_max!{}
  | ^^^^^^^^^^^^^^^^^^^^^^^ expected expression

(the macro generated this, where ◌ is Term::new("", span))

#[inline(never)]
#[no_mangle]
fn() {
    println!("hi there!");
}

fn main() {();
}

Edit: Hm. _ is also on the reserved identifiers list. I wonder...
Edit: ...nope. ◌ is the only reserved identifier that can be constructed by concat_idents!.

@ExpHP
Copy link
Contributor Author

ExpHP commented May 3, 2018

Tʜᴇ Pʟᴏᴛ Tʜɪᴄᴋᴇɴs

You can construct regular keywords as identifiers too:

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

but it does not appear that you can do Unforsakeable Things with this because you can't use the macro in patterns for some reason.

#![feature(concat_idents)]
fn main() {
    let concat_idents!(fn) = 3; // ERROR non-pattern macro in pattern position
    let x = concat_idents!(fn);
}

Obviously my planned simple fix of simply disallowing an empty list would not fix this issue... but is it even an issue? I'm not sure.

Edit: This is not an issue I think, because raw identifiers will enable you to do the same

kennytm added a commit to kennytm/rust that referenced this issue May 3, 2018
Forbid constructing empty identifiers from concat_idents

The empty identifier is a [reserved identifier](https://github.com/rust-lang/rust/blob/8a37c75a3a661385cc607d934c70e86a9eaf5fd7/src/libsyntax_pos/symbol.rs#L300-L305) in rust, apparently used for black magicks like representing the crate root or somesuch... and therefore, being able to construct it is Ungood.  Presumably.

...even if the macro that lets you construct it is so useless that you can't actually do any damage with it. (and believe me, I tried)

Fixes rust-lang#50403.

**Note:** I noticed that when you try to do something similar with `proc_macro::Term`, the compiler actually catches it and flags the identifier as reserved.  Perhaps a better solution would be to somehow have that same check applied here.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant