Bug 20023 – Separate compilation breaks dip1000 / dip1008 @safety

Status
RESOLVED
Resolution
FIXED
Severity
critical
Priority
P1
Component
dmd
Product
D
Version
D2
Platform
All
OS
All
Creation time
2019-07-03T17:02:14Z
Last change time
2021-11-30T10:58:33Z
Keywords
accepts-invalid, industry, pull, safe
Assigned to
No Owner
Creator
John Colvin

Comments

Comment #0 by john.loughran.colvin — 2019-07-03T17:02:14Z
// test.d import test2; void main() { threw!()(); } // test2.d auto threw()() @safe { try throw new Exception("Hello"); catch (Exception e) return e; assert(0); } # dmd test.d -dip1000 -dip1008 -dip25 -c # dmd test2.d -dip1000 -dip1008 -dip25 -c # dmd test.obj test2.obj Happy to compile despite the bug # dmd test.d test2.d -dip1000 -dip1008 -dip25 test2.d(6): Error: scope variable e may not be returned test.d(5): Error: template instance `test2.threw!()` error instantiating Catches the problem correctly
Comment #1 by bugzilla — 2020-03-14T07:56:01Z
This is explained because some of the checks are only done for modules that are present on the command line. You can see this in the line: if (sc._module && sc._module.isRoot() && https://github.com/dlang/dmd/blob/master/src/dmd/escape.d#L1205 You can find the isRoot() check in several places. This is done to ease the transition to dip1000 by not checking code that is only imported, presumably from 3rd party libraries which would be impractical for the user to change himself. Once dip1000 becomes thoroughly propagated to old libraries, this can be tightened up, but not now.
Comment #2 by john.loughran.colvin — 2020-03-14T09:40:04Z
(In reply to Walter Bright from comment #1) > This is explained because some of the checks are only done for modules that > are present on the command line. You can see this in the line: > > if (sc._module && sc._module.isRoot() && > > https://github.com/dlang/dmd/blob/master/src/dmd/escape.d#L1205 > > You can find the isRoot() check in several places. > > This is done to ease the transition to dip1000 by not checking code that is > only imported, presumably from 3rd party libraries which would be > impractical for the user to change himself. > > Once dip1000 becomes thoroughly propagated to old libraries, this can be > tightened up, but not now. Wow, I did not realise that was the case. Does this mean that many things I think are really safe aren't at all? Depending on exactly how I compile my code? That sounds like a nightmare. Unless I'm misunderstanding you, I desperately want to be able to turn this transition aid off immediately and go fix the problems it has been hiding. Also, should this really be closed as wontfix? Presumably it will be fixed, just not yet? Adding industry.
Comment #3 by john.loughran.colvin — 2020-03-16T12:34:35Z
I'm going to reopen as this is presumably going to be fixed, either as a switch or the eventual "tightening up" and in the meantime it's a really nasty thing to trip over.
Comment #4 by john.loughran.colvin — 2020-06-26T13:39:50Z
Discussed on a call with Walter, seems like he misunderstood the original example. To be clear: putting -dip1000 -dip1008 -dip25 on all the modules does not mean all the code is actually checked by those dips.
Comment #5 by bugzilla — 2020-10-09T13:37:02Z
Probably the way to fix this is to add a switch which says make these checks transitive.
Comment #6 by dlang-bot — 2021-11-23T17:54:13Z
@dkorpel created dlang/dmd pull request #13350 "Fix issue 20023 - Separate compilation breaks dip1000 / dip1008 @safety" fixing this issue: - Fix issue 20023 - Separate compilation breaks dip1000 / dip1008 @safety https://github.com/dlang/dmd/pull/13350
Comment #7 by dlang-bot — 2021-11-30T10:58:33Z
dlang/dmd pull request #13350 "Fix issue 20023 - Separate compilation breaks dip1000 / dip1008 @safety" was merged into master: - f3c7b8f9b21ebd6b903ae5270ff04d3bf743ea1e by dkorpel: Fix issue 20023 - Separate compilation breaks dip1000 / dip1008 @safety https://github.com/dlang/dmd/pull/13350