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

operator/ of std::chrono::duration and custom type #40475

Closed
llvmbot opened this issue Mar 18, 2019 · 7 comments
Closed

operator/ of std::chrono::duration and custom type #40475

llvmbot opened this issue Mar 18, 2019 · 7 comments
Assignees
Labels
bugzilla Issues migrated from bugzilla libc++ libc++ C++ Standard Library. Not GNU libstdc++. Not libc++abi.

Comments

@llvmbot
Copy link
Collaborator

llvmbot commented Mar 18, 2019

Bugzilla Link 41130
Resolution FIXED
Resolved on Apr 01, 2019 09:39
Version unspecified
OS All
Attachments code to reproduce, patch by Howard Hinnant
Reporter LLVM Bugzilla Contributor
CC @mclow,@zoecarver

Extended Description

Defining an operator/ with a chrono::duration and a custom type fails to compile with libc++.

Example code attached
Problem description initially on https://stackoverflow.com/q/55167862/620382
Suggested patch by Howard Hinnant https://stackoverflow.com/a/55203313/620382

@llvmbot
Copy link
Collaborator Author

llvmbot commented Mar 18, 2019

assigned to @mclow

@mclow
Copy link
Contributor

mclow commented Mar 18, 2019

Nice patch, Howard. Needs more cowbell, though.
In particular, it needs to deal with operator%.
Working on it...

@mclow
Copy link
Contributor

mclow commented Mar 19, 2019

Howard's patch (when applied twice) fixes the problem; except on C++03.

@mclow
Copy link
Contributor

mclow commented Mar 20, 2019

Multiply has similar problems, too.

@zoecarver
Copy link
Contributor

Are there tests for these operators?

@mclow
Copy link
Contributor

mclow commented Mar 20, 2019

Are there tests for these operators?

There are ... some.
I have added more as I investigate this bug.

@mclow
Copy link
Contributor

mclow commented Apr 1, 2019

Fixed in r357410.

@llvmbot llvmbot transferred this issue from llvm/llvm-bugzilla-archive Dec 10, 2021
rothn added a commit to rothn/libcamera that referenced this issue Oct 14, 2023
A bug in libcxx [0] used by clang version 11.0.2 is prevalent when
building libcamera for Android SDK30. This has been fixed and
integrated upstream with [1].

As a workaround, directly cast libcamera::utils::Duration objects to
std::chrono::duration when dividing.

Alternatives evaluated:
Considered: Enable public inheritance of std::chrono::duration and
override operator/ in the class.
Outcome: Does not fix the original compiler error.

Considered: Enable public inheritance of std::chrono::duration and
override operator/ in the libcamera namespace.
Outcome: new compiler error:
ld.lld: error: duplicate symbol: libcamera::operator/
(libcamera::utils::Duration const&, libcamera::utils::Duration const&)

Considered: Use private inheritance of std::chrono::duration and
re-implement a pass-through version of each std::chrono::duration
operator within libcamera::utils::Duration and use template
metaprogramming to fix the division operator.
Outcome: Testing shows that this would introduce substantial
limitations, i.e. requring the Duration object to be on the LHS of any
arithmetic operation with other numeric types. This also substantially
increases implementation complexity.

Considered: Extract double values from libcamera::utils::Duration
objects and use those to divide.
Outcome: This creates substantial readability and unit-safety issues.

[0] llvm/llvm-project#40475
[1] llvm/llvm-project@efa6d80
Bug: https://bugs.libcamera.org/show_bug.cgi?id=156

Signed-off-by: Nicholas Roth <nicholas@rothemail.net>
This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bugzilla Issues migrated from bugzilla libc++ libc++ C++ Standard Library. Not GNU libstdc++. Not libc++abi.
Projects
None yet
Development

No branches or pull requests

3 participants