Skip to content

Comments

Fix Issue 17712 - [REG 2.074] [LINK] Undefined reference to std.conv.toChars!(10, char, 1, uint).toChars(uint)#6659

Merged
dlang-bot merged 1 commit intodlang:masterfrom
shove70:patch-1
Sep 7, 2018
Merged

Fix Issue 17712 - [REG 2.074] [LINK] Undefined reference to std.conv.toChars!(10, char, 1, uint).toChars(uint)#6659
dlang-bot merged 1 commit intodlang:masterfrom
shove70:patch-1

Conversation

@shove70
Copy link
Contributor

@shove70 shove70 commented Aug 13, 2018

@shove70 shove70 requested a review from JackStouffer as a code owner August 13, 2018 04:06
@dlang-bot
Copy link
Contributor

dlang-bot commented Aug 13, 2018

Thanks for your pull request and interest in making D better, @shove70! We are looking forward to reviewing it, and you should be hearing from a maintainer soon.
Please verify that your PR follows this checklist:

  • My PR is fully covered with tests (you can see the annotated coverage diff directly on GitHub with CodeCov's browser extension
  • My PR is as minimal as possible (smaller, focused PRs are easier to review than big ones)
  • I have provided a detailed rationale explaining my changes
  • New or modified functions have Ddoc comments (with Params: and Returns:)

Please see CONTRIBUTING.md for more information.


If you have addressed all reviews or aren't sure how to proceed, don't hesitate to ping us with a simple comment.

Bugzilla references

Auto-close Bugzilla Severity Description
17712 regression [REG 2.074] [LINK] Undefined reference to std.conv.toChars!(10, char, 1, uint).toChars(uint)

Testing this PR locally

If you don't have a local development environment setup, you can use Digger to test this PR:

dub fetch digger
dub run digger -- build "master + phobos#6659"

@ghost
Copy link

ghost commented Aug 13, 2018

There are other ways to fix it (https://issues.dlang.org/show_bug.cgi?id=17712#c11) which i would prefer. It's delicate because while there's another test case, this is still only a workaround for one of the case of a compiler bug.

Copy link

@ghost ghost left a comment

Choose a reason for hiding this comment

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

This is a delicate issue. Also your PR is not well prepared anyway, title is not good. git commit message neither. "Update conv.d" is too vague.

@n8sh n8sh changed the title Update conv.d Fix Issue 17712 - [REG 2.074] [LINK] Undefined reference to std.conv.toChars!(10, char, 1, uint).toChars(uint) Sep 1, 2018
@n8sh
Copy link
Member

n8sh commented Sep 4, 2018

There are other ways to fix it (https://issues.dlang.org/show_bug.cgi?id=17712#c11) which i would prefer. It's delicate because while there's another test case, this is still only a workaround for one of the case of a compiler bug.

@bbasile I'm sympathetic to this but Issue #17712 has been open for a year without movement. I'm going to mark this 72h no objection -> merge. If another PR is made during that time it can go ahead. Otherwise I think the right thing to do is merge this, and if another solution is made in the future it can replace it.

Copy link
Member

@n8sh n8sh left a comment

Choose a reason for hiding this comment

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

Note: current Win32 failure appears unrelated.

@n8sh n8sh added Merge:72h no objection -> merge The PR will be merged if there are no objections raised. Merge:auto-merge labels Sep 4, 2018
@dlang-bot dlang-bot merged commit 217c9af into dlang:master Sep 7, 2018
ntrel added a commit to ntrel/phobos that referenced this pull request Jun 21, 2020
b17712.d is not fixed in dmd, Phobos workaround:
2f20661
dlang#6659

b17712_c13.d was apparently fixed in dmd:
c2eefb0d8d850715f492197b0b1a033d9ad5411a
dlang/dmd#9636

Add very basic test facility for independent test files.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Merge:auto-merge Merge:72h no objection -> merge The PR will be merged if there are no objections raised. Severity:Bug Fix

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants