Skip to content

Conversation

@ghost
Copy link

@ghost ghost commented Oct 24, 2017

Prevent warnings emitted after a possible compiler change.

@dlang-bot
Copy link
Contributor

Thanks for your pull request, @bbasile! We are looking forward to reviewing it, and you should be hearing from a maintainer soon.

Some tips to help speed things up:

  • smaller, focused PRs are easier to review than big ones

  • try not to mix up refactoring or style changes with bug fixes or feature enhancements

  • provide helpful commit messages explaining the rationale behind each change

Bear in mind that large or tricky changes may require multiple rounds of review and revision.

Please see CONTRIBUTING.md for more information.

Bugzilla references

Your PR doesn't reference any Bugzilla issue.

If your PR contains non-trivial changes, please reference a Bugzilla issue or create a manual changelog.

@ghost ghost requested review from andralex, dnadlinger and ibuclaw as code owners October 24, 2017 14:10
Copy link
Member

@andralex andralex left a comment

Choose a reason for hiding this comment

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

Are there visible improvements in the generated code?

std/numeric.d Outdated
lastRow[i] = 1; // -sin(pi * 3 / 2) == 1
else
lastRow[i] = -sin(i * 2.0L * PI / size);
lastRow[i] = -sin(i * cast(float)(2.0 * PI) / size);
Copy link
Member

Choose a reason for hiding this comment

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

use float(2.0 * PI) here instead

@ghost
Copy link
Author

ghost commented Oct 24, 2017

Improvements are not the point of the PR which is rather to remove the warnings emitted in case dlang/dmd#7240 (comment) would be accepted, but the test failures show that the change cause problems.

@andralex
Copy link
Member

I see, thanks. cc @WalterBright

@ghost ghost requested a review from kyllingstad as a code owner October 24, 2017 15:58
@ghost ghost closed this Oct 28, 2017
@ghost ghost deleted the truncating-conv branch April 23, 2018 13:35
This pull request was closed.
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