Skip to content
This repository was archived by the owner on Oct 12, 2022. It is now read-only.

Revert "Merge pull request #92 from shoo/fix7020" #142

Merged
merged 1 commit into from
Jul 8, 2012

Conversation

MartinNowak
Copy link
Member

This fixes Issue 7375.
It reverts typeinfo name comparison introduced with #92. This fails as the name of the base class is mod.Base!(i) not mod.Base!(22).

See Issue 7020 for further reasons why this has been the wrong fix in the first place.

This reverts commit 29f33bf, reversing
changes made to f11dd3e.

fixes Issue 7375
@WalterBright
Copy link
Member

The compiler shouldn't have used the same name for the different instantiations. That fixes the immediate problem, but whether throwing exceptions across DLL boundaries by checking the name strings is a good idea or not is still open to question.

@MartinNowak
Copy link
Member Author

Exceptions are only a small part of the problem that arise when having multiple runtimes in one process.
Think of stdio Files that use different locks. Adding string comparison was as wrong as adding the GC proxy.
Both are hacky workarounds for breaking the ODR but none of them helps us to make phobos a DLL.

@alexrp
Copy link
Member

alexrp commented May 14, 2012

What's the status of this pull request? The Bugzilla issue is marked as fixed.

@MartinNowak
Copy link
Member Author

In an attempt to fix a symptom of ODR violation #92 introduced class name comparison in _d_isbaseof2.
This is both slow and wrong. Slow because it adds many string comparisons.
Wrong because having the same type name doesn't imply type equivalence, e.g. different versions of a class coexisting in a process (when hotswapping instances) are part of different type hierarchies.
There is more trivia in Bugzilla 7020.

andralex added a commit that referenced this pull request Jul 8, 2012
Revert "Merge pull request #92 from shoo/fix7020"
@andralex andralex merged commit 047fa7c into dlang:master Jul 8, 2012
rainers pushed a commit to rainers/druntime that referenced this pull request Jun 9, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants