Deprecate the FxHashMap() and FxHashSet() constructor function hack

[oli-obk]

No description provided.

[rust-highfive]

r? @nikomatsakis

(rust_highfive has picked a reviewer for you, use r? to override)

[petrochenkov]

Nit: I'd personally use Default::default() instead of SpecificMapOrSetType::default() where possible, types change from time to time and with Default::default() you don't have to adjust initializers.
Also, fewer imports.

[oli-obk]

This is possible in struct construction cases but not when local variables are initialized and immediately used by calling e.g. insert.

I will adjust all use sites accordingly

[oli-obk]

Also, imports won't get fewer, as most of the time you need the type name for the struct fields

[RalfJung]

I'd personally use Default::default() instead of SpecificMapOrSetType::default() where possible, types change from time to time and with Default::default() you don't have to adjust initializers.

Uh, I'd prefer if you wouldn't do that change. That's throwing away useful type information, which can already be hard to get by with all the inference we are doing. It doesn't make code more readable.

We should optimize for reading the code, which happens all the time, not changing the type, which only happens rarely.

[RalfJung]

So, is this fully replacing #52591? Looks to be just a partial successor?

[oli-obk]

Looks to be just a partial successor?

Yes that's on purpose. Otherwise I'll never stay ahead of the bitrot curve

[oli-obk]

So... how do I continue? 😆

@petrochenkov are you fine with not changing everything to Default::default? (well, everything that can be changed, which is around 3/4th I guess)

[petrochenkov]

@oli-obk
Can you do it in field initializers?
That's where it doesn't mess with type inference and is not harmful for readability.

[RalfJung]

is not harmful for readability.

I still think this does more harm then good -- now I have to scroll to the struct definition to figure out the types. That can be arbitrarily far away.

But I won't fight over this.

[nikomatsakis]

FWIW, in my own projects, the rule I follow is usually:

• When implementing Default (or a new function) I use Default::default()
  • this is partially because it lets me write simpler Rust macros, emacs macros, or otherwise move quickly through this boilerplate stuff
• Otherwise, I prefer Type::default() for the reasons @RalfJung and @oli-obk gave -- I find it more readable, and I don't get surprises when I tweak the code and no longer have sufficient type hints

[nikomatsakis]

r=me with whichever conclusion we reach.

[bors]

☔ The latest upstream changes (presumably #54671) made this pull request unmergeable. Please resolve the merge conflicts.

[sanmai-NL]

Ad @nikomatsakis in #55114 (comment):
These kinds of idiom conventions are ephemeral if they’re not written down in https://github.com/rust-lang-nursery/api-guidelines. Discussing it in pull requests won’t capture the knowledge very well and discoverably.

[nikomatsakis]

@sanmai-NL true. But that particular comment was just giving my preferences. In any case, I think such docs might be appropriate for rustc-guide -- feels like it doesn't belong in the API guidelines (particularly since these decisions don't affect the API very much).

(That said, I do think there is an API decision here, in that I've come to favor implementing Default over implementing a zero-argument new fn.)

[nikomatsakis]

@bors r+

[bors]

📌 Commit 8892efb25d88d37da63ae434d3150da4c405e1d5 has been approved by nikomatsakis

[oli-obk]

@bors r=nikomatsakis p=1

bitrot priority

[bors]

📌 Commit 53e92f4 has been approved by nikomatsakis

[bors]

⌛ Testing commit 53e92f4 with merge 81b0c05...

[bors]

💔 Test failed - status-appveyor

[RalfJung]

Looks like it timed out after 3h.

@bors retry

[bors]

⌛ Testing commit 53e92f4 with merge 528939ce01557bc516a4993e81be9d0ff8a19e85...

[bors]

💔 Test failed - status-appveyor

[kennytm]

@bors retry AWS S3 Outage in US-WEST-1.

[bors]

⌛ Testing commit 53e92f4 with merge 94273f4...

[bors]

☀️ Test successful - status-appveyor, status-travis
Approved by: nikomatsakis
Pushing 94273f4 to master...