Skip to content

Conversation

monarchdodra
Copy link
Collaborator

Basically reverts #1390 now that we have a more usable doesPointTo.

Pinging @DmitryOlshansky and @andralex.

I removed a unittest that was just kind of wrong (swapping with internal pointers), which I had introduced, but I'm not sure why...

@monarchdodra
Copy link
Collaborator Author

WTF are these failures?

@monarchdodra
Copy link
Collaborator Author

Hum... the version(assert) seems to be responsible for the error, but the code seems legit to me. Am I right in thinking this is a bug?

@dnadlinger
Copy link
Contributor

@monarchdodra: It might be an issue with the linked Phobos library being built with different flags than the program. DMD likely expects that the missing symbols were emitted into libphobos already and skips them, but when the library is built, assertions are not enabled.

I think the proper fix for this is to ship two copies of Phobos, one used for release builds, and another with assertions in Phobos code enabled.

@monarchdodra
Copy link
Collaborator Author

@monarchdodra: It might be an issue with the linked Phobos library being built with different flags than the program. DMD likely expects that the missing symbols were emitted into libphobos already and skips them, but when the library is built, assertions are not enabled.

Hum... I figured as much, but we're dealing with templates here, so I'm surprised to see that this would happen. I'd expect the compiler to know if/if-not a symbol needs to be compiled and linked?

I think the proper fix for this is to ship two copies of Phobos, one used for release builds, and another with assertions in Phobos code enabled.

Arguably, that's a workaround?

@dnadlinger
Copy link
Contributor

Arguably, that's a workaround?

@monarchdodra: Well, in the general case, one can not expect to be able to link against a library that has been compiled with completely different -version=… flags. Of course, one could still argue that -release is a special case, usability-wise…

@monarchdodra
Copy link
Collaborator Author

Alright, I removed the offending version(assert). This should be good to go now. @DmitryOlshansky ?

DmitryOlshansky added a commit that referenced this pull request May 31, 2014
reinsert doesPointTo in swap/move
@DmitryOlshansky DmitryOlshansky merged commit fdf5307 into dlang:master May 31, 2014
@CyberShadow
Copy link
Member

This pull request may have introduced a regression:
http://forum.dlang.org/post/vldwvnuivsfwjozqiuup@forum.dlang.org

@monarchdodra monarchdodra deleted the swapPointsTo branch August 29, 2014 10:54
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.

4 participants