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

Removes pow(complex,int) overload #1299

Merged
merged 3 commits into from
Oct 3, 2020

Conversation

MattStephanson
Copy link
Contributor

@MattStephanson MattStephanson commented Sep 16, 2020

Fixes #1260. Test still doesn't run cleanly, unfortunately, due to implicit narrowing conversion in test code.

Removes pow(complex,int) overload, per N4861 [cmplx.over]/3.

  • Restrict overloads to complex and arithmetic types.
  • _Uglify existing type aliases.

 - Restrict overloads to complex and arithmetic types.
 - _Uglify exiting type aliases.
@MattStephanson MattStephanson marked this pull request as ready for review September 16, 2020 17:51
@MattStephanson MattStephanson requested a review from a team as a code owner September 16, 2020 17:51
@StephanTLavavej StephanTLavavej added the bug Something isn't working label Sep 16, 2020
@cbezault cbezault removed their assignment Sep 30, 2020
Copy link
Member

@StephanTLavavej StephanTLavavej left a comment

Choose a reason for hiding this comment

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

Thanks for fixing this longstanding bug! Everything looks good to me, except for the pre-existing mess that we've gotten into with Gaussian integers. Since that pow overload isn't helping conformance (and indeed complicates overload resolution for your fix), and the basic scenario of pow(complex<int>, int) has always been broken, I'll push a change to remove that overload.

stl/inc/complex Outdated Show resolved Hide resolved
This scenario has always been broken.
Copy link
Member

@StephanTLavavej StephanTLavavej left a comment

Choose a reason for hiding this comment

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

I verified that the STL builds and the libcxx test directory passes (for Clang which doesn't emit the warning) with the Gaussian integer pow removed.

@cbezault - I pushed this change after you approved.

@CaseyCarter - This is just to say

I have final-reviewed
the complex pow PR
that was assigned to you
on Wednesday

and which
you were probably
saving
for Friday

Forgive me
@MattStephanson's fix was delicious
so conformant
and so much removed complexity

@StephanTLavavej StephanTLavavej self-assigned this Oct 2, 2020
@StephanTLavavej StephanTLavavej merged commit d05f8ec into microsoft:master Oct 3, 2020
@StephanTLavavej
Copy link
Member

Thanks again for making this <complex> code simple! 😹

@MattStephanson MattStephanson deleted the gh1260_pow_over branch January 1, 2021 07:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

<complex>: pow, incorrect type conversion
4 participants