Upgrade rust to 1.57

[tarikeshaq]

Upgrades rust to 1.57

The changes here were mechanical and based on rustc or clippy's recommendations...

Two main highlights:

• We now use rust stable for the cross-compilation of the M1 simulators, not urgent but if CI passes @skhamis it would be awesome if you can give this a shot when you have the time 🙏 🙏
• rustc now considers code that is written, but never read "dead code" I found at least one instance where it got it wrong (there was an assert_eq that definitely read that field 😅) so I opted to allow them across the board, except for the example code. Thoughts?

Pull Request checklist

• Quality: This PR builds and tests run cleanly
  • Note:
    • For changes that need extra cross-platform testing, consider adding [ci full] to the PR title.
    • If this pull request includes a breaking change, consider cutting a new release after merging.
• Tests: This PR includes thorough tests or an explanation of why it does not
• Changelog: This PR includes a changelog entry in CHANGES_UNRELEASED.md or an explanation of why it does not need one
  • Any breaking changes to Swift or Kotlin binding APIs are noted explicitly
• Dependencies: This PR follows our dependency management guidelines
  • Any new dependencies are accompanied by a summary of the due dilligence applied in selecting them.

[codecov-commenter]

Codecov Report

Merging #4723 (596d0ff) into main (f000e81) will not change coverage.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##             main    #4723   +/-   ##
=======================================
  Coverage   80.53%   80.53%           
=======================================
  Files          48       48           
  Lines        5220     5220           
=======================================
  Hits         4204     4204           
  Misses       1016     1016           

Impacted Files	Coverage Δ
components/nimbus/src/persistence.rs	95.07% <100.00%> (ø)

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update f000e81...596d0ff. Read the comment docs.

[mhammond]

there's not alot of value here, but I think this is reading a desktop places row, which possibly has some value? Maybe add a leading underscore on the names (and a comment about why?) I'm assuming this is what is described in rust-lang/rust#81658, and that's one of the suggestions there?)

[bendk]

I think I have a slight preference for commenting out fields like this rather than deleting it, adding the underscore, or adding the #[allow(dead_code)] annotation. This seems like a nice way to indicate that the fields are present in the source data in case we want to use them some time in the future. That said, I'm fine with any of the solutions.

[mhammond]

that new dead_code check is unfortunate :( But we can't just stay on the old rust, so we have to do something. I'm fine with this, but I'll leave it for others to weigh in and approve and/or suggest changes.

[bendk]

rustc now considers code that is written, but never read "dead code" I found at least one instance where it got it wrong (there was an assert_eq that definitely read that field)

Was the assert_eq in a test? Maybe we should update the test code or just delete the test. There doesn't seem to be much use in testing something that never gets read in the actual code.

[tarikeshaq]

Was the assert_eq in a test?

Yeah, it's stuff like

application-services/components/places/src/storage/bookmarks.rs

Lines 2113 to 2127 in 021e649

	assert_eq!(bm.sync_change_counter, 0);
	// Update to an empty string sets it to null
	update_bookmark(
	&conn,
	&guid,
	&UpdatableBookmark {
	title: Some("".to_string()),
	..Default::default()
	}
	.into(),
	)?;
	let bm = get_raw_bookmark(&conn, &guid)?.expect("should exist");
	assert_eq!(bm.title, None);
	assert_eq!(bm.sync_change_counter, 1);

I'm not too sure about all that's going on there, but the sync_change_counter seems to be used as some type of sanity checking? Same goes for the other dead_code fields I assigned for that type, think the other structs I dead_codeed are fine thou

[mhammond]

huh - so in that particular example, it looks like the sync_change_counter field isn't actually read by the implementation - it's just read by sql, so apart from those tests, could actually be removed from the struct. Still don't have a strong opinion about the best way to deal with that though :)

[mhammond]

Still don't have a strong opinion about the best way to deal with that though :)

I guess for that particular case, a #cfg might work, which might be an interesting experiment, but I'm completely fine with a pragmatic decision that it's not worthwhile here.

[tarikeshaq]

a #cfg might work

👀 👀 👀 Love the idea!! I'll give it a shot

[mhammond]

lgtm!