search

cessen / ropey

A utf8 text rope for manipulating and editing large texts.
MIT License
1.04k stars 46 forks source link

Incoming breakage due to static assert on size #56

Closed oli-obk closed 2 years ago

oli-obk commented 2 years ago

https://github.com/rust-lang/rust/pull/94075 optimizes repr(Rust) enums better, making them smaller in many cases. This breaks ropey (see https://crater-reports.s3.amazonaws.com/pr-94075/try%23419e70b02dc61a51433bb55fe5a482c6ab6f7da5/gh/cessen.led/log.txt for an example). I'm not sure when this change gets merged and will hit stable, but relying on the exact size of repr(Rust) types is suboptimal. The easy fix is to use repr(C) for the affected enums, which will disable any such optimizations.

If you have opinions on this topic, please don't hesitate to let me know here or post on the PR.

cessen commented 2 years ago

Ah! Thanks for the heads up. I'll look into this soon.

cessen commented 2 years ago

@oli-obk: I've pushed what I believe is a fix to master in commit dafa0b69f87d006154d29ef17bd8b84293927611. If you could try building it with https://github.com/rust-lang/rust/pull/94075 and let me know if it's actually fixed, I'd really appreciate it! Then I can cut a new Ropey release with the fix.

oli-obk commented 2 years ago

You can try this out locally by using https://crates.io/crates/rustup-toolchain-install-master to install the toolchain with hash 419e70b02dc61a51433bb55fe5a482c6ab6f7da5

cessen commented 2 years ago

Ah, thanks! That's convenient. I assumed I would have to build the whole thing myself, which... sounded quite daunting, especially just to test it out.

cessen commented 2 years ago

Looks like it's fixed. Thanks!

I'll leave this issue open until I cut a release with the fix.

cessen commented 2 years ago

Published in Ropey 1.5.0.