Skip to content

Add std::chrono::time_point constructor overloads and tests - #50

Open
yalagadapavankumar wants to merge 3 commits into
rustfoundation:mainfrom
yalagadapavankumar:constructor-overloading
Open

yalagadapavankumar wants to merge 3 commits into
rustfoundation:mainfrom
yalagadapavankumar:constructor-overloading

Conversation

@yalagadapavankumar

@yalagadapavankumar yalagadapavankumar commented Oct 13, 2026 •

Copy link
Copy Markdown

Add C++ std::chrono::time_point constructors in Rust using overload! macro.

  • Default constructor: Creates time point at the clock's epoch.
  • Constructor from milliseconds: Creates time point from duration in milliseconds.
  • Converting constructor: Creates time point from another time point that uses seconds.
  1. Write Rust wrappers for that C++ type, for 3-5 different overloaded method names:
    • Create a new file named after your type in overloading-macros/cpp-overload-test/src/stdcpp
    • Wrap constructors first, writing tests to make sure the correct constructor is called
    • Explain any potential compatibility issues in a Limitations doc section
    • Explain usability issues in an Ergonomics doc section
    • Check that you have implemented all the overloads for each method name, or documented why you can't
    • Push your changes and wait for a review
  2. Add a separate example with some failing overloads
    • Create a new file named after your type in overloading-macros/cpp-overload-test/examples
    • Wrap accessor/modifier methods
    • Explain how they fail in an Incompatibilities doc section
    • Explain how the overload failures could be resolved in a Workarounds doc section
    • Optional: If there's a C++ feature with no equivalent in Rust, explain that in an Inexpressible Overloads doc section
    • Push your changes and wait for a review
  3. Update the overload macro compatibility table Markdown doc
    • If you haven't checked some kinds of overloads, put a ?in those squares
    • Push your changes and wait for a review

teor2345

This comment was marked as resolved.

Comment thread cpp-overload-test/src/stdcpp/time_point.rs
@yalagadapavankumar

This comment was marked as resolved.

@teor2345

This comment was marked as resolved.

@yalagadapavankumar

Copy link
Copy Markdown
Author

Hi teor, i thought using just checklist is more convenient, as it makes easier to what completed.

Do you think it's better to include whole PR template?

@yalagadapavankumar

This comment was marked as resolved.

@teor2345

Copy link
Copy Markdown
Collaborator

for the October round, and as far as I can see, they lead to the October channels.

this is the link representing as text which is not for October channel, which is exits in all 3 rust projects, for this project Extend Rust stdlib custom allocator support, they didn't provide zulip properly, so people believe this text is zulip channel for this project.

This is now fixed, and if people accidentally introduce themselves somewhere else, someone from the community will move their post to the right channel.

@teor2345 teor2345 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, the instructions weren't clear, we want to see 3-5 different overloaded function names, wrapping all the overloads for each name.

I have updated the PR template and the checklist at the top of this PR with the new wording.

You can choose whatever overloaded functions you like, but I suggest:

#include <chrono>

using TestClock = std::chrono::system_clock;
using TestDuration = std::chrono::milliseconds;

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This specific type alias makes the code harder to read, because it hides that the type is milliseconds.
Instead, delete this alias, and use the milliseconds type directly.

Please also rename TestTimePoint so it is clear it is in milliseconds.


/// A wrapper around a C++ time point with millisecond precision.
#[cfg_attr(not(test), expect(dead_code, reason = "Only used in tests"))]
pub struct StdTimePoint(*mut c_void, PhantomData<()>);

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As above, hiding the units makes the type error-prone to use.

Suggested change
pub struct StdTimePoint(*mut c_void, PhantomData<()>);
pub struct StdMillisecondsTimePoint(*mut c_void, PhantomData<()>);

#[cfg_attr(not(test), expect(dead_code, reason = "Only used in tests"))]
impl StdSecondsTimePoint {
/// Construct a time point from seconds since the epoch.
fn from_seconds(seconds: i64) -> Self {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The i64 to long long type conversion may fail on some platforms. Instead, use the Rust type that is guaranteed to be a long long on all platforms:
https://doc.rust-lang.org/std/ffi/type.c_longlong.html

Please make this change throughout this file.

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