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

Change #[lang="foo"]-style items to #[lang("foo")] #11886

Closed
pcwalton opened this issue Jan 28, 2014 · 21 comments
Closed

Change #[lang="foo"]-style items to #[lang("foo")] #11886

pcwalton opened this issue Jan 28, 2014 · 21 comments
Labels
A-parser Area: The parsing of Rust source code to an AST
Milestone

Comments

@pcwalton
Copy link
Contributor

Only one ("string") literal allowed.

@lilyball
Copy link
Contributor

Is this just to simplify the syntax for attributes? Personally, I like seeing #[crate_id="foo"] because it looks like I'm assigning "foo" to crate_id, and that's what I'm trying to do. #[crate_id("foo")] looks like I'm trying to invoke some functionality named crate_id with the value "foo".

@pcwalton
Copy link
Contributor Author

It also lets us remove the brackets. #crate_id="foo" looks weird, but #crate_id("foo") doesn't.

@lilyball
Copy link
Contributor

I like the brackets!

Removing the brackets means giving up the potential future use of # as an operator outside of attributes. Has this been considered? The meeting discussion seems to take it as given that removing brackets is a good thing. I also note that 3 months ago I commented about this same issue, and got no response there either.

Also, regarding the one literal restriction, doesn't that cause problems for deriving? #[deriving(Eq, Clone)] is pretty common right now.

@pnkfelix
Copy link
Member

@kballard the word "literal" in "one literal restriction" is referring solely to string literals. All existing attribute syntax that doesn't use key="value string" would continue to work, I believe.

@lilyball lilyball reopened this Jan 28, 2014
@lilyball
Copy link
Contributor

Sorry, misclick.

@brson
Copy link
Contributor

brson commented Jan 28, 2014

I'd rather use #[lang(foo)] since lang item names can just be identifiers.

@brson
Copy link
Contributor

brson commented Jan 28, 2014

@kballard The problem with brackets is that we also want to add the bang to inner attributes, which means the syntax for inner attributes is #![foo];, which is a heck of a lot of syntax.

@lilyball
Copy link
Contributor

@brson Why not drop the semicolon in that case? #![foo] seems reasonable.

And I agree that #[lang(foo)] is appropriate for that attribute. But attributes like crate_id, crate_type, license, etc. make more sense using the #[crate_id="foo"] syntax.

@brson
Copy link
Contributor

brson commented Jan 28, 2014

@kballard @pnkfelix likes the semicolon. This is a discussion for #2569.

@brson
Copy link
Contributor

brson commented Jan 28, 2014

Maybe we should have a discussion on the mailing list about the complete plan for attributes before going through with this.

Personally, I also like the brackets.

@brson
Copy link
Contributor

brson commented Jan 28, 2014

If we just leave attribute syntax alone it will prevent a lot of work, at the expensive of people forever wtfing over the semicolon distinction.

@glaebhoerl
Copy link
Contributor

Ever since the idea came up I've been writing attributes as #attr and then having to correct myself to add the brackets.

@pnkfelix
Copy link
Member

Accepted for 1.0, P-backcompat-lang.

@brson
Copy link
Contributor

brson commented Jan 31, 2014

I think I've made my peace with foo("bar") as a replacement for foo = "bar", independent of whether we remove brackets. There's something appealing about just using s-expressions for attributes if we can.

@fhahn
Copy link
Contributor

fhahn commented Feb 7, 2014

Is there a consensus for this issue?

Should it be foo("bar") ? I think this would require changes to the attribute parsing, because the item expected between the () seems to be an ident.

I've created a quick proof of concept patch that uses lang(foo) but I did not convert the existing lang items yet.

@brson
Copy link
Contributor

brson commented Feb 15, 2014

@fhahn Yes, let's do it. It does require changes to the parser.

@fhahn
Copy link
Contributor

fhahn commented Feb 15, 2014

Okay, so the parser should be extended to accept strings in items, like lang("foo")?

I'll be away for a week or so, so I'll probably won't get much work done next week.

@brson
Copy link
Contributor

brson commented Feb 17, 2014

@fhahn yes.

@alexcrichton
Copy link
Member

Nominating for closure. I think with #12538 we don't need to change this

@pnkfelix
Copy link
Member

closing; there are some proponents for doing this but the presented arguments are not really strong enough to continue discussing it. (e.g. some arguments involve coupling in other changes to the attribute syntax, such as switching to @ for attributes)

@glaebhoerl
Copy link
Contributor

Are there any compelling arguments for having the brackets other than historical inertia?

flip1995 pushed a commit to flip1995/rust that referenced this issue May 3, 2024
…1995

Fix `FormatArgs` storage when `-Zthreads` > 1

Fixes rust-lang#11886

The initial way I thought of was a little gross so I never opened a PR for it, I thought of a nicer way today that no longer involves any `thread_local`s or `static`s

`rustc_data_strucutres::sync::{Lrc, OnceLock}` implement `DynSend` + `DynSync` so we can pass them to the lint passes that need the storage

changelog: none

r? `@flip1995`
flip1995 pushed a commit to flip1995/rust that referenced this issue May 3, 2024
…1995

Fix `FormatArgs` storage when `-Zthreads` > 1

Fixes rust-lang#11886

The initial way I thought of was a little gross so I never opened a PR for it, I thought of a nicer way today that no longer involves any `thread_local`s or `static`s

`rustc_data_strucutres::sync::{Lrc, OnceLock}` implement `DynSend` + `DynSync` so we can pass them to the lint passes that need the storage

changelog: none

r? `@flip1995`
xFrednet pushed a commit to xFrednet/rust that referenced this issue May 5, 2024
…1995

Fix `FormatArgs` storage when `-Zthreads` > 1

Fixes rust-lang#11886

The initial way I thought of was a little gross so I never opened a PR for it, I thought of a nicer way today that no longer involves any `thread_local`s or `static`s

`rustc_data_strucutres::sync::{Lrc, OnceLock}` implement `DynSend` + `DynSync` so we can pass them to the lint passes that need the storage

changelog: none

r? `@flip1995`
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-parser Area: The parsing of Rust source code to an AST
Projects
None yet
Development

No branches or pull requests

7 participants