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

Add new TimSpan overload support coming from .NET 9 #3994

Merged
merged 7 commits into from
Jan 8, 2025

Conversation

MangelMaxime
Copy link
Member

@MangelMaxime

This comment was marked as resolved.

@MangelMaxime MangelMaxime force-pushed the fix/timespan_overload_net9 branch from 8eebc37 to ed4bb77 Compare January 2, 2025 20:51
@MangelMaxime
Copy link
Member Author

The remaining failing CI probably needs #3960, we should be able to work around this limitation by using compiler directives.

@MangelMaxime
Copy link
Member Author

MangelMaxime commented Jan 3, 2025

I found out that the Rust target create one function per overload, this can be seen here for example:

pub fn new_ticks(ticks: i64) -> TimeSpan {
TimeSpan { ticks: ticks }
}
pub fn new_hms(h: i32, m: i32, s: i32) -> TimeSpan {
let hoursTicks = (h as i64) * ticks_per_hour;
let minsTicks = (m as i64) * ticks_per_minute;
let secTicks = (s as i64) * ticks_per_second;
Self::new_ticks(hoursTicks + minsTicks + secTicks)
}
pub fn new_dhms(d: i32, h: i32, m: i32, s: i32) -> TimeSpan {
let hours = d * 24 + h;
Self::new_hms(hours, m, s)
}
pub fn new_dhms_milli(d: i32, h: i32, m: i32, s: i32, ms: i32) -> TimeSpan {
let hours = (d * 24 + h) as i64;
let hoursTicks = hours * ticks_per_hour;
let minsTicks = (m as i64) * ticks_per_minute;
let secTicks = (s as i64) * ticks_per_second;
let msTicks = (ms as i64) * ticks_per_millisecond;
Self::new_ticks(hoursTicks + minsTicks + secTicks + msTicks)
}

I will do something similar for supporting the new TimeSpan overload.

@MangelMaxime
Copy link
Member Author

@ncave Is ./build.sh quicktest rust working for you ?

When trying to run it I have this error:

error[E0599]: no function or associated item named `try_seconds` found for struct `chrono::Duration` in the current scope
   --> /workspaces/Fable/temp/fable-library-rust/src/./DateTime.rs:70:39
    |
70  |         let subsecond = d - Duration::try_seconds(seconds).unwrap();
    |                                       ^^^^^^^^^^^ function or associated item not found in `Duration`
    |

Even if the IDE in VSCode is able to find the try_seconds function.

@ncave
Copy link
Collaborator

ncave commented Jan 5, 2025

@MangelMaxime

Is ./build.sh quicktest rust working for you ?

Fixed in #4001

@ncave
Copy link
Collaborator

ncave commented Jan 7, 2025

@MangelMaxime

The remaining failing CI probably needs #3960, we should be able to work around this limitation by using compiler directives.

Yes, using FABLE_COMPILER_5 should work. You can merge it as is, I can add the Rust part in a separate PR if you prefer.

@MangelMaxime MangelMaxime force-pushed the fix/timespan_overload_net9 branch from ed4bb77 to 009b223 Compare January 8, 2025 10:22
@MangelMaxime
Copy link
Member Author

Yes, using FABLE_COMPILER_5 should work. You can merge it as is, I can add the Rust part in a separate PR if you prefer.

Thank you, I had a look at adding Rust support but this is not add straightforward as it seemed to me. I will look at your PR to learn from it.

@MangelMaxime MangelMaxime merged commit 75b5165 into main Jan 8, 2025
16 of 18 checks passed
@MangelMaxime MangelMaxime deleted the fix/timespan_overload_net9 branch January 8, 2025 10:32
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.

2 participants