-
Notifications
You must be signed in to change notification settings - Fork 13.2k
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
derive: clean up hygiene #32251
derive: clean up hygiene #32251
Conversation
This replaces some `if`s with `match`es. This was originally not possible because using a global path in a match statement caused a "non-constant path in constant expr" ICE. The issue is long since closed, though you still hit it (as an error now, not an ICE) if you try to generate match patterns using pat_lit(expr_path). But it works when constructing the patterns more carefully.
This changes local variable names in all derives to remove leading double-underscores. As far as I can tell, this doesn't break anything because there is no user code in these generated functions except for struct, field and type parameter names, and this doesn't cause shadowing of those. But I am still a bit nervous.
When deriving Hash, RustcEncodable and RustcDecodable, the syntax extension needs a type parameter to use in the inner method. They used to use __H, __S and __D respectively. If this conflicts with a type parameter already declared for the item, bad times result (see the test). There is no hygiene for type parameters, but this commit introduces a better heuristic by concatenating the names of all extant type parameters (and prepending __H).
⌛ Testing commit 8355389 with merge 3f5db0e... |
@bors: retry force clean |
⌛ Testing commit 8355389 with merge 1efa752... |
derive: clean up hygiene derive: clean up hygiene Fixes #2810. Spawned from #32139. r? @alexcrichton
This broke warnings: #![deny(warnings)]
#[derive(Hash)]
struct Foo;
fn main() {} will now throw a warning/error about an unused argument. Please keep the |
Bleh, you're right. The reason was assumed to be hygiene, but it's also On Wed, Mar 16, 2016 at 2:41 PM, Manish Goregaokar <notifications@github.com
|
Yep. Double underscore is slightly better IMO since clippy uses that to distinguish between things that are silenced because they may be unused (e.g. when they come from a macro), and things which are known to be unused. But single underscore is also ok. |
See #32292 |
Should the fix just re-add underscores to the idents introduced in this PR? I'm not entirely sure if that's all that needs to be done. |
Yeah, I should just re-add double underscores I think. The other improvements in this PR are turning an if-else into match (was a FIXME in the code) and making it harder for the derived type parameter to cause a collision, neither of which are critical. |
Cool. I'm working on it. |
Oh okay, well thanks for cleaning up my mess :) On Wed, Mar 16, 2016 at 2:51 PM, Manish Goregaokar <notifications@github.com
|
derive: clean up hygiene
Fixes #2810.
Spawned from #32139.
r? @alexcrichton