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

[WIP] feat(std): add time module to standard library #779

Open
wants to merge 26 commits into
base: master
Choose a base branch
from

Conversation

Etherian
Copy link
Contributor

@Etherian Etherian commented Aug 24, 2019

Description

Adds a time module to the standard library which exposes the functionality of Rust's std::time.

Status

In Development

Outstanding design questions

  • How should std.time be structured -- std.time.Instant.now, std.time.instant_now, std.instant.now, or something else? (std.time.Instant.now)
  • Should std.time be behind a feature? Should that feature be default? (not behind feature)

To Do

  • Implement all functionality in std::time
    • Implement types
    • Implement functions
    • Implement implicit interfaces (Eq and Ord)
  • Add documentation
    • Generated std lib docs
    • Examples
    • In Rust code? (no need)
  • Add tests (no need)

Future Directions

  • add duration.from_secs_float, duration.as_secs_float, duration.mul_float, and duration.div_float
  • Add more full-featured date-and-time functionality to the standard library? Chrono?

@Etherian
Copy link
Contributor Author

Etherian commented Aug 24, 2019

Here's my rough draft of a std.time module. Structurally, I'm leaning towards each of the three types getting their own namespace -- e.g. std.time.Instant.now or std.time.Duration.checked_sub.

For now, I'm stuck on an error that occurs when I attempt to import std.time.

> .\target\debug\gluon.exe -i
gluon (:h for help, :q to quit)
> let time = import! std.time
error: error: Userdata cannot be cloned
- <std.time>:3:20
  |
3 | let time = import! std.time.prim
  |                    ^^^^^^^^^^^^^
  |

Any suggestions?

src/std_lib/time.rs Outdated Show resolved Hide resolved
src/std_lib/time.rs Outdated Show resolved Hide resolved
@Marwes
Copy link
Member

Marwes commented Aug 25, 2019

Here's my rough draft of a std.time module. Structurally, I'm leaning towards each of the three types getting their own namespace -- e.g. std.time.Instant.now or std.time.Duration.checked_sub.

👍 std.time.duration.checked_sub etc

@Marwes
Copy link
Member

Marwes commented Aug 25, 2019

Should std.time be behind a feature? Should that feature be default?

I don't think a feature is warranted for modules mirroring rusts std library. If we end up seeing that gluon's standard library is to large at some point it could be investigated then.

@Etherian
Copy link
Contributor Author

Adding #[gluon_userdata(clone)] caused a couple errors.

error[E0053]: method `deep_clone` has an incompatible type for trait
  --> src\std_lib\time.rs:16:24
   |
16 | #[derive(Clone, Debug, Userdata, Trace, VmType)]
   |                        ^^^^^^^^ expected struct `vm::gc::Borrow`, found struct `vm::gc::GcPtr`
   |
   = note: expected type `fn(&std_lib::time::Duration, &'gc mut vm::api::Cloner<'_>) -> std::result::Result<vm::gc::Borrow<'gc, vm::gc::GcPtr<std::boxed::Box<(dyn vm::api::Userdata + 'static)>>>, vm::Error>`
              found type `fn(&std_lib::time::Duration, &mut vm::api::Cloner<'_>) -> std::result::Result<vm::gc::GcPtr<std::boxed::Box<(dyn vm::api::Userdata + 'static)>>, vm::Error>`

error[E0308]: mismatched types
  --> src\std_lib\time.rs:16:24
   |
16 | #[derive(Clone, Debug, Userdata, Trace, VmType)]
   |                        ^^^^^^^^
   |                        |
   |                        expected struct `vm::gc::GcPtr`, found struct `vm::gc::Borrow`
   |                        expected `std::result::Result<vm::gc::GcPtr<std::boxed::Box<(dyn vm::api::Userdata + 'static)>>, vm::Error>` because of return type
   |
   = note: expected type `std::result::Result<vm::gc::GcPtr<std::boxed::Box<(dyn vm::api::Userdata + 'static)>>, _>`
              found type `std::result::Result<vm::gc::Borrow<'_, vm::gc::GcPtr<std::boxed::Box<dyn vm::api::Userdata>>, >, _>`

@Etherian
Copy link
Contributor Author

Could you mentor me regarding tests for this module? I don't have much experience writing tests and I am not confident I can write them correctly and thoroughly. Maybe you could give me examples or an outline of what properties to test and how to test those properties?

Also, how do I include documentation for external types and values?

@Marwes
Copy link
Member

Marwes commented Sep 15, 2019

Since this is mostly just exposing functions defined and tested by rust itself I don't think the testing needs to be so thorough. Having some examples like in https://doc.rust-lang.org/std/time/struct.Duration.html would be good though.

There isn't a way to define documentation in rust code so you will need to define it in the gluon module

/// Docs
let new = prim.new
{
    new,
    /// Docs
    checked_sub = prim.checked_sub,
    ....
}

Doc tests require some unfortunate boilerplate atm, but you can look at the json module for how they are done

gluon/std/json/de.glu

Lines 67 to 77 in a507ed0

/// Deserializes a `Bool`
///
/// ```
/// let { ? } = import! std.effect
/// let { Value, bool, deserialize_with } = import! std.json.de
/// let { Result, ? } = import! std.result
/// let { assert_eq, ? } = import! std.test
///
/// seq assert_eq (deserialize_with bool "true") (Ok True)
/// assert_eq (deserialize_with bool "123") (Err "Expected bool")
/// ```

@Etherian
Copy link
Contributor Author

I finally got around to finishing the doc comments for std.time. Is there a way I can test the examples without running the whole doc test suite?

@Marwes
Copy link
Member

Marwes commented Aug 15, 2020

cargo test --test main --features test -- @time should only compile and run tests for modules matching time

@Etherian
Copy link
Contributor Author

It looks like doc comments are not currently recognized on record entries, which is where I have placed my docs for the time modules. Could this behavior be added, and/or should I restructure my gluon files so doc comments are all attached to let bindings?

Marwes added a commit to Marwes/gluon that referenced this pull request Aug 16, 2020
@Marwes
Copy link
Member

Marwes commented Aug 16, 2020

#870

bors bot added a commit that referenced this pull request Aug 16, 2020
870: test: Extract doc tests defined in record fields r=Marwes a=Marwes

cc #779

Co-authored-by: Markus Westerlind <marwes91@gmail.com>
@Etherian
Copy link
Contributor Author

Etherian commented Sep 6, 2020

I'm running into a few errors while testing the examples.


This line in some examples

let dur_show : Show duration.Duration = { show = \_ -> "<duration>" }

seems to cause the following error:

thread 'tokio-runtime-worker' panicked at 'Lambda without source location. Please report an issue at https://github.com/gluon-lang/gluon/issues', check\src\rename.rs:246:94

Replacing the lambdas with const fixed these errors.


Various other errors occur unpredictably. In fact, one time all the tests passed without any of these errors. I don't have any guesses as to why they are occurring.

thread 'tokio-runtime-worker' panicked at 'assertion failed: `(left == right)`
  left: `7`,
 right: `3`', vm\src\stack.rs:579:9
thread 'tokio-runtime-worker' panicked at 'called `Result::unwrap()` on an `Err` value: "PoisonError { inner: .. }"', vm\src\thread.rs:1143:22
thread 'tokio-runtime-worker' panicked at 'Attempted to pop Frame { offset: 22, state: Extern(ExternState { function: std.time.prim.system_time.duration_since 0x5dcb40, call_state: Poll, locked: None }), excess: false } but std.time.prim.system_time.now was expected
thread 'tokio-runtime-worker' panicked at 'Expected extern: Closure(ClosureState { closure: ClosureData { function: 0x12cc4750:wrap@38_5, upvars: [] }, instruction_index: 0 }). Please report an issue at https://github.com/gluon-lang/gluon/issues', vm\src\stack.rs:251:18
thread ':tokio-runtime-worker' panicked at ':ValueRef is not an Userdata: Int(0). Please report an issue at https://github.com/gluon-lang/gluon/issues', <rustlib>/src/rust\src\libstd\macros.rs:16:9

Full output of a test run
> cargo test --test main --features test time
    Finished test [unoptimized + debuginfo] target(s) in 1.33s
     Running target\debug\deps\main-79fe047da3eb48b4.exe
main
        doc
                std.time.duration
                        Duration ... ok
                        new ... ok
                        from_secs ... ok
                        from_millis ... ok
                        from_micros ... ok
                        from_nanos ... ok
                        as_secs ... ok
                        subsec_millis ... ok
                        subsec_micros ... ok
                        subsec_nanos ... ok
                        as_millis ... ok
                        as_micros ... thread 'tokio-runtime-workerok' panicked at '
Expected extern: Closure(ClosureState { closure: ClosureData { function: 0x1438a1d0:assert_some@59_5, upvars: [<0x12d29420:wrap@38_5 0x141afa28>, {2147483648: {2147483648: <0x12d84980:std.effect.functor@35_11 0x142b1a28>}, {2147483648: {2147483648: <0x12d84980:std.effect.functor@35_11 0x142b1a
                        as_nanos ... 28>}, <0x1413ddc0:std.effect.applicative@40_13 0x142b1d28>, <0x14088e80:wrap_eff@28_5 0x142b1728>}, {2147483648: {2147483648: {2147483648: }, <0x1413ddc0:std.effect.applicative@40_13 0x142b1d28>, <0x14088e80:wrap_eff@28_5 0x142b1728>}, <0x14088ee0:flat_map_eff@29_5 0x142b1828>}, <0x13ed0020:ruokn_pure@56_5 0x142b1f28>, <0x13ed0000:inject_rest@51_5 0x142b1e28>}, <0x140fdb90:tell@22_5 0x14394ea8>] }, instruction_index: 0 }). Please report an issue at https://github.com/gluon-lang/gluon/issues
', vm\src\stack.rs                      checked_add ... :ok251
:18                     checked_sub ...
thread 'oktokio-runtime-workernote: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

' panicked at 'Attempted to pop Frame { offset: 22, state: Extern(ExternState { function: std.time.prim.system_time.duration_since 0x5dcb40, call_state: Poll, locked: None }), excess: false } but std.time.prim.system_time.now was expected                     checked_mul ... ', okvm\src\thread.rs
:1927                   checked_div ... :thread 'ok13tokio-runtime-worker

' panicked at 'called `Result::unwrap()` on an `Err` value: "PoisonError { inner: .. }"
thread '', tokio-runtime-workervm\src\thread.rs         std.time.instant
' panicked at ':called `Result::unwrap()` on an `Err` value: "PoisonError { inner: .. }"1143                    Instant ... ', :okvm\src\thread.rs22
:
1143                    now ... :FAILED22

thread 'thread 'tokio-runtime-workerExpected extern: Closure(ClosureState { closure: ClosureData { function: 0x1438a1d0:assert_some@59_5, upvars: [<0x12d29420:wrap@38_5 0x141afa28>, {2147483648: {2147483648: <0x12d84980:std.effect.functor@35_11 0x142b1a28>}, {2147483648: {2147483648: <0x12d84980:std.effect.functor@35_11 tokio-runtime-worker' panicked at '0x142b1a28>}, <0x1413ddc0:std.effect.applicative@40_13 0x142b1d28>, <0x14088e80:wrap_eff@28_5 0x142b1728>}, {2147483648: {2147483648: {2147483648: }, <0x1413ddc0:std.effect.applicative@40_13 0x142b1d28>, <0x14088e80:wrap_eff@28_5 0x142b1728>}, <0x14088ee0:flat_map_eff@29_5 0x142b1828>}, <0x13e' panicked at 'called `Result::unwrap()` on an `Err` value: "PoisonError { inner: .. }"d0020:run_pure@56_5 0x142b1f28>, <0x13ed0000:inject_rest@51_5 0x142b1e28>}, <0x140fdb90:tell@22_5 0x14394ea8>] }, instruction_index: 0 }). Please rcalled `Result::unwrap()` on an `Err` value: "PoisonError { inner: .. }"', eport an issue at https://github.com/gluon-lang/gluon/issues', vm\src\thread.rs                      durvm\src\thread.rsation_since ... ::1143FAILED1143:
:2222

thread 'thread 'tokio-runtime-workercalled `Result::unwrap()` on an `Err` value: "PoisonError { inner: .. }"tokio-runtime-worker' panicked at '
        saturating_duration_since ... ' pacalled `Result::unwrap()` on an `Err` value: "PoisonError { inner: .. }"nicked at 'FAILED', called `Result::unwrap()` on an `Err` value: "PoisonError { inner: .. }"
vm\src\thread.rs', :GetOffset on <0x12d29420:wrap@38_5 0x141afa28>vm\src\thread.rs1143:                 elapsed ... :114322FAILED:

22thread '
tokio-runtime-worker' panicked at 'thread 'called `Result::unwrap()` on an `Err` value: "PoisonError { inner: .. }"tokio-runtime-worker', ' panicked at 'vm\src\thread.rscalled `Result::unwrap()` on an `Err` value: "PoisonError { inner: .. }":1143', vm\src\thread.rs::221143:
22
thread 'called `Result::unwrap()` on an `Err` value: "PoisonError { inner: .. }"tokio-runtime-worker                    checked_add ... ' panicked FAILEDat '
called `Result::unwrap()` on an `Err` value: "PoisonError { inner: .. }"', vm\src\thread.rscalled `Result::unwrap()` on an `Err` value: "PoisonError { inner: .. }":                       checked_sub ... 1143:FAILED22

called `Result::unwrap()` on an `Err` value: "PoisonError { inner: .. }"
                std.time.system_time
                        unix_epoch ... FAILED
Attempted to pop Frame { offset: 22, state: Extern(ExternState { function: std.time.prim.system_time.duration_since 0x5dcb40, call_state: Poll, locked: None }), excess: false } but std.time.prim.system_time.now was expected                    now ... FAILED
called `Result::unwrap()` on an `Err` value: "PoisonError { inner: .. }"                        duration_since ... FAILED
called `Result::unwrap()` on an `Err` value: "PoisonError { inner: .. }"                        elapsed ... FAILED
called `Result::unwrap()` on an `Err` value: "PoisonError { inner: .. }"                        checked_add ... FAILED
called `Result::unwrap()` on an `Err` value: "PoisonError { inner: .. }"                        checked_sub ... FAILED
called `Result::unwrap()` on an `Err` value: "PoisonError { inner: .. }"


test result: FAILED. 18 passed; 12 failed; 175 filtered
Some tests failed
error: test failed, to rerun pass '--test main'

@Etherian
Copy link
Contributor Author

Etherian commented Oct 3, 2020

@Marwes Have you had a chance to look at this?

@Marwes
Copy link
Member

Marwes commented Oct 8, 2020

Sorry, no. Haven't had that much motivation to code outside of work lately. Won't have time over the weekend either but I will try to look at it next week.

@Etherian
Copy link
Contributor Author

Etherian commented Oct 9, 2020

Sure. No rush. 🙂

@Etherian
Copy link
Contributor Author

Etherian commented Aug 2, 2021

The good news is no conflicts have cropped up. The bad news is that it's still throwing errors I don't know how to solve. Do you have time to take a look, @Marwes?

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