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 style-servo to avoid spurious rebuilds #240

Merged
merged 1 commit into from
May 29, 2018
Merged

Change style-servo to avoid spurious rebuilds #240

merged 1 commit into from
May 29, 2018

Conversation

Mark-Simulacrum
Copy link
Member

The old code generation generated different code on rebuilds due to
depending on HashSet iteration order; this patch patches that problem by
replacing the HashSet with a BTreeSet. --pretty=expanded output is now
identical across builds.

cc rust-lang/rust#48184, @SimonSapin @michaelwoerister

I'm not actually sure this fixes the entire problem, but it's probably a good start.

The old code generation generated different code on rebuilds due to
depending on HashSet iteration order; this patch patches that problem by
replacing the HashSet with a BTreeSet. --pretty=expanded output is now
identical across builds.
@SimonSapin
Copy link
Contributor

Looks good to me. Would you please also send a PR upstream for the same change, with a code comment explaining that iteration order affects rebuilds?

@Mark-Simulacrum
Copy link
Member Author

Hm, my understanding is that upstream had a larger rewrite that made this unnecessary? Or at least so you indicate in this comment; I don't see any similar looking code in the master version of cg.rs.

@SimonSapin
Copy link
Contributor

Oh that’s right, never mind then :)

@Mark-Simulacrum Mark-Simulacrum merged commit 6c6a061 into rust-lang:master May 29, 2018
@Mark-Simulacrum Mark-Simulacrum deleted the variance-reduce branch May 29, 2018 17:46
@michaelwoerister
Copy link
Member

Thanks, @Mark-Simulacrum!

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

Successfully merging this pull request may close these issues.

3 participants