-
Notifications
You must be signed in to change notification settings - Fork 61
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
Compilation error when trying to use palette #283
Comments
I had similar errors that came and went away but I don't remember how I fixed it last time. Which compiler version are you using? Does it change if you switch to another version? |
I updated to the current nightly during our conversation on the weekend. My code unfortunately requires |
I see, then it's trickier to test with something stable unless it can be minimized into an example without them. I'm not sure off the top of my head but you may be able to pick the nightly version with rustup. |
Using an old nightly (in this case the one from 2022-01-01) didn't help. Neither did replacing the division by 2 by a multiplication by 0.5. This was the only line, the compiler didn't like, though. And extreme disambiguation did help
|
What if you write |
I'm reopening this because there may be something that can mitigate the issue. |
Yes.
Still does not work |
There is an answer in the StackOverflow post, which I originally linked, which seems to contain 2 workarounds. But I didn't read them properly. |
Ah, so something about how it transforms the division into a function call is getting stuck. It would be nice to have a small test case to keep here and also find out which implementation is the problematic one. Would you mind sharing the full code or try to reduce it into a minimal test case? |
I'll have a look at those workarounds later and see if they could help too. Thanks for pointing them out! |
/~https://github.com/NeverGivinUp/palette_compilation_rest.git (was meant to be a compilation test, not a rest) |
Perfect, thanks! 🙏 |
This just started hitting my code today. Any multiplication with two f64's trips up one by one. I can't go change all these lines to the ::mul(self, rhs) format, since that would be silly. What would you recommend to do? |
Ouch, sorry about that. You can try to use variables with explicit types to help the compiler, but it's also not ideal. 😕 You could also see if it depends on the compiler version. I didn't get around to figuring out exactly what triggers it before releasing, so I'm sorry for not having better answers right now, but thanks for highlighting that it's still an issue. |
For me, it happens in a projects after running a routinely Updating crates.io index
Adding fast-srgb8 v1.0.0
Removing find-crate v0.6.3
Updating getrandom v0.2.8 -> v0.2.9
Updating palette v0.6.1 -> v0.7.0
Updating palette_derive v0.6.1 -> v0.7.0
Updating ratio-color v0.2.0 -> v0.3.0
Updating serde v1.0.159 -> v1.0.160
Updating serde_derive v1.0.159 -> v1.0.160
Updating simba v0.8.0 -> v0.8.1
Updating syn v2.0.13 -> v2.0.14
Removing toml v0.5.11
Updating uuid v1.3.0 -> v1.3.1 Where The first offending line in my code is a const multiplication of: pub const DOUBLE_PI: f64 = 2.0_f64 * std::f64::consts::PI; But as soon as I patch that, all f64 multiplications inside function bodies get marked, probably because of the order in which things are compiled. |
It's rather uncommon for a library to touch the functionalities of others (or std in that matter), which makes it a rather strange error that should have been prohibited by the orphan rule usually? |
Indeed, there's no obvious reason why it would be bothered by Do you think it would be possible to make a minimal example of when it fails? It could be cross referenced with the one above to see if there's any pattern. Also, which compiler version are you using? |
I am able to reproduce this the following way: [package]
name = "bug_palette"
version = "0.1.0"
edition = "2021"
[dependencies]
scad = "1.2.2"
palette = "0.7.0" // main.rs
use palette::Oklch;
use scad::OffsetType;
fn main() {
println!("{}", 42.0 * 1.0); // bug also happens when specifying f32 or f64
}
I'm on latest stable:
|
Great, thanks! From this it pretty much looks like it's getting stuck while considering I will give it a proper try this weekend (or tomorrow if I have time) |
By the way @Azorlogh, does it still fail when removing the |
The issue is quite spooky indeed. Removing any of the |
Definitely spooky but the spookiness also makes it harder to set up a reliable test case for regression. 🤔 |
I found this, that seem to be pretty much the same case: rust-lang/rust#80542 |
I created PR #312 to try to work around this. It works for the example from @Azorlogh, but let me know if it still fails in any other cases. You should be able to change your [dependencies]
palette = { git = "/~https://github.com/Ogeon/palette.git", branch = "work_around_issue_283" } |
A minimal example might be quite difficult as the application I'm working on is a decently large GUI using If you still would like to have a look I could push the current state of my repo to a new branch and send you a link in a couple of hours. |
That would also be helpful, thanks. Do you know if the problem started after any particular change? |
The problem started when updating to the latest version of iced, I fear that it may be some other bug that causes this and makes the compile go a little haywire. I committed my working changes here /~https://github.com/Smalands-Nation/register-rs/tree/iced_migration_palette_bug |
Thank you. If it's anything like the original issue, it's basically a latent problem that just didn't get triggered until now. A trait requirement that happens to send the trait resolver into a loop when the stars happen to align. I'm asking all these questions because it's so difficult to reproduce it without those triggering circumstances. |
It's ultimately a compiler bug, and I hoped the next resolver would be able to handle it (it does handle some cases of it), but I'll have to find some workaround that isn't too breaking for now. |
I feared as much... I've only taken a very quick glance but couldn't this be solved for some to the types by introducing a trait bound on the inner iterator that is not met by the colors themselves. A hasty example for Rgb could look something like impl<'a, S, C> IntoIterator for Rgb<S, C>
where
C: IntoIterator + num_traits::Num,
{
...
} Of course this assumes the inner iterator is never another colortype which I do not know is always the case as I've only skimmed through |
That's also my prime suspect. The feature that introduced the iterator is the ability to convert an array of colors into a struct of arrays, so |
Could we not then impose bounds on Another hasty example impl<'a, S, C> IntoIterator for Rgb<S, C>
where
C: IntoIterator,
<C as IntoIterator>::Item: num_traits::Num,
{
...
} |
The library isn't relying on |
I tried restricting the item type, but it didn't help. I will make implementations for specific collection types instead. |
I'm also running into this while attempting to upgrade use {
std::convert::Infallible,
iced::{
Application,
Command,
Element,
Settings,
widget::Column,
},
};
struct State;
impl Application for State {
type Executor = iced::executor::Default;
type Message = Infallible;
type Theme = iced::Theme;
type Flags = ();
fn new((): Self::Flags) -> (Self, Command<Infallible>) { (Self, Command::none()) }
fn title(&self) -> String { String::default() }
fn update(&mut self, _: Infallible) -> Command<Infallible> { Command::none() }
fn view(&self) -> Element<'_, Infallible> {
Column::with_children(Vec::default().into_iter().map(|()| Column::new()).collect()).into()
}
}
fn main() -> iced::Result {
State::run(Settings::default())
} |
Oh, thanks for the minimal example! I'll see if I can make it into a test later. |
I have pushed a preliminary work-around as #380. The solution for [patch.crates-io]
palette = { git = "/~https://github.com/Ogeon/palette", branch = "issue_283_into_iterator" } |
With this patch applied, the compiler seems to get stuck (2 minutes of CPU time and counting on the example above, 19 minutes and counting on the original project). |
That's probably what I thought was slow linking, but I had to cancel it and go. 😬 It could be the |
I upgraded my app to iced 0.12 and i have the same issue with Luv overflow evaluating the requirement |
I added a commit with less generic implementations for You should get the new commit with |
Looks good! |
That's great! If nothing more pops up, I'll finish it next time I have time (this week is a bit cramped 😬) and publish it. |
Version 0.7.5 has been released with the fix. I will remove the temporary branch and you can remove the Cargo.toml patch. Thanks for reporting the error and testing the fix! |
I'm still getting a really similar error to the ones mentioned above |
@Ogeon Thanks for the fix. That made it apparent what the issue was. On 0.7.3, I got the recursion error as above, and on 0.7.5 I got an error like this: error[E0283]: type annotations needed
--> proj/src/lib.rs:123:68
|
123 | Column::with_children(Vec::default().into_iter().map(|()| Column::new()).collect()).into()
| --------------------- ^^^^^^^ cannot infer type of the type parameter `B` declared on the method `collect`
| |
| required by a bound introduced by this call
|
= note: cannot satisfy `_: IntoIterator`
note: required by a bound in `iced::widget::Column::<'a, Message, Theme, Renderer>::with_children` If I change the code to use |
Hi, that looks like a regular ambiguity that can happen with I'm assuming this is the function you are calling, which takes a generic type that can be iterated ( |
@Ogeon I'm able to reproduce this recursion limit error when specifying 2148c1148
< version = "0.7.3"
---
> version = "0.7.5"
2150c2150
< checksum = "b2e2f34147767aa758aa649415b50b69eeb46a67f9dc7db8011eeb3d84b351dc"
---
> checksum = "ebfc23a4b76642983d57e4ad00fb4504eb30a8ce3c70f4aee1f725610e36d97a" I can reliably toggle the recursion limit error by switching back and forth between those two versions, and AFAICT no other crates are changing versions. That is the function I'm calling, yeah. Leaving off |
True, it was most likely masking the error. It's possible that it interacted badly with |
When adding any use statement from my local palette crate to my library -- I have not tried using the official version -- I get the following error:
The error goes away, when I comment out the line
impl_scalar_binop!(Div::div, DivAssign::div_assign, [f32, f64]);
I believe is unrelated to my local changes and it certainly is unrelated to the code, where the error occurs. It may be related to a bug in the rust compiler.
The text was updated successfully, but these errors were encountered: