Bug 14301 – [2.067-rc1] Private symbols of module conflicts with public from another

Status
RESOLVED
Resolution
FIXED
Severity
regression
Priority
P1
Component
dmd
Product
D
Version
D2
Platform
x86_64
OS
Linux
Creation time
2015-03-17T14:41:00Z
Last change time
2017-07-19T17:42:42Z
Keywords
pull
Assigned to
nobody
Creator
NCrashed

Comments

Comment #0 by NCrashed — 2015-03-17T14:41:51Z
Compiles on 2.066.1: test.d: ``` module test; class Cache {} ``` app.d: ``` import test; import std.algorithm; void main() { Cache cache; } ``` Output: ``` source/app.d(6): Error: module std.algorithm struct std.algorithm.iteration.Cache(R, bool bidir) is private source/app.d(6): Error: test.Cache at source/test.d(3) conflicts with std.algorithm.iteration.Cache(R, bool bidir) at /usr/include/dmd/phobos/std/algorithm/iteration.d(295) ```
Comment #1 by public — 2015-03-17T15:35:02Z
I don't see what can be done here to be honest. This is inherent flaw of D module system - adding new symbols to Phobos/druntime will break user code that already defined symbols with same name,
Comment #2 by schuetzm — 2015-03-17T16:14:53Z
This problem's been around since D1 times. There's a PR [1] that fixes this at least partially (apart from overloading AFAIU). [1] https://github.com/D-Programming-Language/dmd/pull/3743 *** This issue has been marked as a duplicate of issue 1238 ***
Comment #3 by dlang-bugzilla — 2015-03-18T10:36:23Z
The compiler bug is one thing, but does that mean we should ignore regressions introduced in Phobos that are caused by the compiler bug? The std.algorithm symbol could instead be renamed or made scope-local, if possible.
Comment #4 by dlang-bugzilla — 2015-03-18T10:37:17Z
(In reply to Dicebot from comment #1) > I don't see what can be done here to be honest. This is inherent flaw of D > module system - adding new symbols to Phobos/druntime will break user code > that already defined symbols with same name, std.algorithm.iteration.Cache is private, so we can rename it to something less likely to collide with user symbols without affecting user code.
Comment #5 by public — 2015-03-18T10:38:56Z
> std.algorithm.iteration.Cache is private Ah, I have missed that. Yes, renaming can be an option in that case. However it does not really address the core issue - adding any new public symbol to Phobos may break the code and we won't be able to provide any reasonable migration path for that (apart from possible dfix rule)
Comment #6 by dlang-bugzilla — 2015-03-18T10:40:05Z
Yeah, I know. Let's do what we can for now.
Comment #7 by dlang-bugzilla — 2015-03-18T12:15:15Z
Comment #8 by ketmar — 2015-03-18T14:25:28Z
(In reply to Marc Schütz from comment #2) > This problem's been around since D1 times. There's a PR [1] that fixes this > at least partially (apart from overloading AFAIU). there is Kenji's import system rewrite[1], which fixes this, and many other things. yes, it will break some code that relies on current invalid implementation, but this *must* be done sooner or later. better do it sooner. so far it's "opened for discussion" by Walter, which effectively means "we can safely forget about it, nothing will be done or even discussed anymore". [1] https://github.com/D-Programming-Language/dmd/pull/3416
Comment #9 by code — 2015-03-19T01:38:31Z
(In reply to Ketmar Dark from comment #8) > there is Kenji's import system rewrite[1], which fixes this, and many other > things. yes, it will break some code that relies on current invalid > implementation, but this *must* be done sooner or later. better do it sooner. > > so far it's "opened for discussion" by Walter, which effectively means "we > can safely forget about it, nothing will be done or even discussed anymore". It's a bit more complex than that. There are 3 different import "bugs", issue 313, issue 314, and the visibility of private symbols (http://wiki.dlang.org/DIP22, https://github.com/D-Programming-Language/dmd/pull/739). Kenji made a nice fix for issue 313, but conflated it with a fix for issue 314 that removes aliases for selective imports. The latter is problematic, we had to revert the last 314 fix because of this. It's also highly debatable to remove the aliases and by this time a lot of people rely on them. It would be much simpler to fix 314 by correctly applying protection. https://github.com/D-Programming-Language/dmd/pull/3416#issuecomment-40041165 https://github.com/D-Programming-Language/dmd/pull/3407#issuecomment-66280773 The way imports need to be fixed is fairly clear by now.
Comment #10 by ketmar — 2015-03-19T02:15:08Z
(In reply to Martin Nowak from comment #9) > It's also highly > debatable to remove the aliases and by this time a lot of people rely on > them. this is the root of the problem, yes. this is exactly why c++ is crap: "we can't change our old decisions and fix broken things, 'cause there are alot of people who are using those things!" this is one of the reasons why D is so little used after many years of developement. D has alot of great features, and some broken features. that broken features *can* be fixed, there is no committee that has to accept new stadard. and there are not so much vital D code. some of that features aren't even in specs. yet instead of fixing that features here and now there is a continuous process of augmenting that features with various workarounds. yes, those fixes will break some code. but refusing to fix 'em will leave features broken forever, as there are more and more code that rely on broken things. i'm using Kenji's patch for a long time now, and it works perfectly. yes, there are some broken code in various D projects, and that code is *really* broken, it works by accident (due to flaws in module system). most project authors happily accepts patches that adds missing imports, and code becomes better and with better "import locality". yes, fixing that code requires some effort. but migrating from one version of compiler to another version is not seamless too, so i can't see this as a real blocker. it should be done once, and it will solve a wide range of import bugs for future versions. Kenji's work is complex, but it fixes imports once and forever. more than that, is makes D better and easier to use. i believe that D is still in position where D developers can think more about future users than about not breaking the code that will breaks anyway after two compiler releases.
Comment #11 by gassa — 2015-03-19T08:23:05Z
Meanwhile, and a bit off topic, is there a convenient tool to do just that: diagnose missing imports? I asked on D.learn but received no response so far: http://forum.dlang.org/thread/[email protected]
Comment #12 by ketmar — 2015-03-19T15:55:36Z
you can build dmd with the patch i mentioned and then simply use it to compile your code. ;-)
Comment #13 by ketmar — 2015-03-19T15:56:25Z
ah, sorry, i see what you mean. no, i don't think that there is such tool yet.
Comment #14 by code — 2015-03-20T01:49:12Z
> yes, those fixes will break some code. but refusing to fix 'em will leave features broken forever, as there are more and more code that rely on broken things. https://yourlogicalfallacyis.com/strawman This is not what I was saying, Kenji's patch fixes 313 and 314, which is great, but it also unnecessarily changes how selective imports work. This will break a lot of code to make a behavior different not better. Nobody is against fixing imports, but it's a core language feature and has to be done carefully.
Comment #15 by code — 2015-03-20T01:54:18Z
Comment #16 by ketmar — 2015-03-20T02:13:58Z
(In reply to Martin Nowak from comment #14) > This is not what I was saying, Kenji's patch fixes 313 and 314, which is > great, but it also unnecessarily changes how selective imports work. it fixes *all* issues with selective imports. it *does* *not* contradicts specs. what is wrong then?
Comment #17 by github-bugzilla — 2015-03-20T06:37:23Z
Commits pushed to master at https://github.com/D-Programming-Language/phobos https://github.com/D-Programming-Language/phobos/commit/de7100e7f3fc70f530554b5c09f4dc0e6e2195da fix Issue 14301 - private symbol Cache conflicts with client code https://github.com/D-Programming-Language/phobos/commit/c856128dd84787989291ab38873181f177d1aa89 Merge pull request #3081 from MartinNowak/fix14301 fix Issue 14301 - private symbol Cache conflicts with client code
Comment #18 by github-bugzilla — 2015-03-20T12:19:55Z
Commit pushed to 2.067 at https://github.com/D-Programming-Language/phobos https://github.com/D-Programming-Language/phobos/commit/d7ea0ef8e31e5c431821e079416c56c90c6f3c8e Merge pull request #3081 from MartinNowak/fix14301 fix Issue 14301 - private symbol Cache conflicts with client code
Comment #19 by github-bugzilla — 2015-04-11T12:25:12Z
Comment #20 by github-bugzilla — 2017-07-19T17:42:42Z
Commits pushed to dmd-cxx at https://github.com/dlang/phobos https://github.com/dlang/phobos/commit/de7100e7f3fc70f530554b5c09f4dc0e6e2195da fix Issue 14301 - private symbol Cache conflicts with client code https://github.com/dlang/phobos/commit/c856128dd84787989291ab38873181f177d1aa89 Merge pull request #3081 from MartinNowak/fix14301