Bug 16265 – unittest imports should not be counted as dependencies for static ctors

Status
NEW
Severity
enhancement
Priority
P4
Component
dmd
Product
D
Version
D2
Platform
All
OS
All
Creation time
2016-07-11T13:27:43Z
Last change time
2024-12-13T18:48:55Z
Assigned to
No Owner
Creator
Steven Schveighoffer
See also
https://issues.dlang.org/show_bug.cgi?id=16673
Moved to GitHub: dmd#19157 →

Comments

Comment #0 by schveiguy — 2016-07-11T13:27:43Z
A module like this: shared static this() {...} unittest { import other; // other imports this module, and contains static ctors other.foo(); } Should not be considered a cycle. The shared ctor cannot possibly call the unit test code, so there is no leaking of the import. The only place static ctors can exist inside a unittest is in a local class or struct. We can easily detect this and allow the dependency based on that. This allows things like examples and other useful mechanisms, to be located in the right module, without causing frivolous cycles. I've had to fix several such "cycles" in phobos recently. The easiest way to fix this is to simply not emit the dependency into the module info in such a case.
Comment #1 by dfj1esp02 — 2016-10-03T10:10:54Z
But you will still have a cycle when compiling unittests? Then unittest dependencies can't be possibly ignored.
Comment #2 by code — 2016-10-04T21:42:25Z
> unittest { > import other; // other imports this module, and contains static ctors > other.foo(); > } > > Should not be considered a cycle. The shared ctor cannot possibly call the > unit test code, so there is no leaking of the import. Well, unfortunately it's technically possible using `__traits(getUnitTests, __MODULE__)`, and it wouldn't be too far-fetched to call the tests from a static ctor. The example is pretty close to do that http://dlang.org/spec/traits.html#getUnitTests.
Comment #3 by schveiguy — 2016-10-05T15:46:22Z
unittest dependencies cannot be called directly by static constructors (except by Martin's corner case method), so their imports don't count when doing construction. By the time unittests run, static ctors are already done, so no worries there. (In reply to Martin Nowak from comment #2 > Well, unfortunately it's technically possible using `__traits(getUnitTests, > __MODULE__)`, > and it wouldn't be too far-fetched to call the tests from a static ctor. I disagree this is not far-fetched, do we need to screw up everyone's cycle detection code due to this rare case? Isn't it enough to just say "WARNING unit test imports are not considered during static construction, calling unit tests during static construction can result in undetected cycles"? Consider that documented unit tests are meant to show how one can use a library call, and this might involve importing many other modules that are totally unrelated. Including unittest imports when doing cycle detection is harmful in many ways, it is very advantageous to get rid of this. Please, open a new bug report for other ideas of cycle detection improvement, don't override the purpose of this one.
Comment #4 by schveiguy — 2017-05-07T21:31:30Z
Comment #5 by slavo5150 — 2017-12-12T02:12:50Z
I think it would help to have an example that someone could quickly cut and paste into their development environment to reproduce and illustrate the problem.
Comment #6 by schveiguy — 2017-12-12T21:32:51Z
(In reply to Mike Franklin from comment #5) > I think it would help to have an example that someone could quickly cut and > paste into their development environment to reproduce and illustrate the > problem. Of course. It's going to be a toy example, but things like documented unit tests cause these kinds of problems, especially when you have so many modules that you need stuff from in order to run a complete program. mod1.d: module mod1; immutable int val; shared static this() { val = 2; } size_t bar(size_t x) { return x + 1; } unittest { import mod2; assert("abc".foo.bar == 7); } mod2.d: module mod2; import mod1; immutable int multiplier; shared static this() { multiplier = val; } size_t foo(string s) { return s.length * multiplier; }
Comment #7 by robert.schadek — 2024-12-13T18:48:55Z
THIS ISSUE HAS BEEN MOVED TO GITHUB https://github.com/dlang/dmd/issues/19157 DO NOT COMMENT HERE ANYMORE, NOBODY WILL SEE IT, THIS ISSUE HAS BEEN MOVED TO GITHUB