Bug 102 – Forward reference nested class wheel.

Status
RESOLVED
Resolution
FIXED
Severity
major
Priority
P2
Component
dmd
Product
D
Version
D1 (retired)
Platform
All
OS
All
Creation time
2006-04-11T14:42:00Z
Last change time
2014-02-15T02:09:28Z
Keywords
patch, rejects-valid
Assigned to
nobody
Creator
dawid.ciezarkiewicz
Blocks
340

Attachments

IDFilenameSummaryContent-TypeSize
457defer.patchmore aggressive deferral of (possible) forward declarationstext/plain4653
459defer2.patchupdated patchtext/plain5561
537defer.patchupdated patch againtext/plain7920

Comments

Comment #0 by dawid.ciezarkiewicz — 2006-04-11T14:42:31Z
$ cat test.d import test2; class X { Y.NY t; static class NX { } } $ cat test2.d import test; class Y { X.NX nx; static class NY { } } $ dmd -c test2.d test.d test2.d(3): class test2.Y is forward referenced when looking for 'NY' test2.d(3): class test2.Y is forward referenced when looking for 'NY' (...) $ dmd -c test.d test2.d test.d(3): class test.X is forward referenced when looking for 'NX' test.d(3): class test.X is forward referenced when looking for 'NX' (...)
Comment #1 by dawid.ciezarkiewicz — 2006-07-31T02:35:24Z
$ cat test1.d import test2; class X { Y.NY t; enum NX { BLA, BLA1 } } $ cat test2.d import test1; class Y { X.NX nx; enum NY { FOO, BAR } } Is impossible to compile just like bug #102.
Comment #2 by davidl — 2007-01-23T05:15:15Z
umm, let the compiler solve this kind of forward reference is painful i guess. d 1.0 doesn't let it through
Comment #3 by smjg — 2007-02-11T20:21:29Z
*** Bug 912 has been marked as a duplicate of this bug. ***
Comment #4 by clugdbug — 2009-09-03T00:53:52Z
The original bug, and also the test case in bug 912, were fixed after 1.041 and before 1.047. They work in D2 as well. Only the 'enum' case in comment 2 remains unfixed!
Comment #5 by clugdbug — 2009-09-03T06:49:57Z
This is the slightly reduced test case that still fails. It only needs one file. class X { Y.NY t; enum NX { BLA, BLA1 } } class Y { enum NY { FOO, BAR } X.NX nx; }
Comment #6 by r.sagitario — 2009-09-18T01:33:06Z
Created attachment 457 more aggressive deferral of (possible) forward declarations DMD has a mechanism to defer semantic analysis of declarations, but uses it only sometimes when it detects a forward reference. Often you cannot tell if it is a forward reference or a typo, so I have made this work more aggressively. Details: A new state variable (tryfwd - better name suggestion welcome) is added to the global struct, that switches the behaviour for VarDeclaration::semantic: if analysis of the type produces an error, further analysis is deferred. Error messages are suppressed by the gag-mechanism. The same state variable is used to defer struct size analysis. After the first semantic analysis step on all files on the command line, tryfwd is cleared, and semantic analysis is run again on all deferred symbols, producing appropriate error messages. The downside of this patch is that it might have a negative impact on compilation speed and memory consumption: - depending on the modules layout semantic analysis on deferred symbols is run very often or maybe not often enough. - To avoid overwriting type information with erronuous info, a syntaxCopy is run before the type analysis. This might need a lot of memory, but most types are small or even do not create any copy at all (as the basic types). - error messages are less sequential with respect to lexical order.
Comment #7 by r.sagitario — 2009-09-18T01:36:02Z
This patch also fixes issues 282, 400, 461, 1564, 2386, 2654 and 2666. But before adding this info to these bugs, I'd like some feedback on the patch.
Comment #8 by clugdbug — 2009-09-18T13:39:03Z
IMHO this is a VERY promising patch since it potentially fixes many difficult issues. It's not quite complete though. I've run it though Walter's test suite, and one test fails (all others pass). The code below should fail to compile, but instead the compiler gets into an infinite loop --- it's an ice-on-invalid-code regression. ---- struct A (T) { mixin B! (T, A! (T)); } A!(int) x; ----
Comment #9 by r.sagitario — 2009-09-19T05:52:22Z
Created attachment 459 updated patch Thanks for running the test-suite. The updated patch adds two changes: - when the speculative type semantics fail, "dprogress" is also reset to its original value, so the runDeferredSemantic does not think it should continue. - when template instantiation fails, it is now also removed from the module/class member list. DMD 2.032 just removes it from the list of template instantiations.
Comment #10 by clugdbug — 2009-09-19T12:32:08Z
(In reply to comment #9) > Created an attachment (id=459) [details] > updated patch > > Thanks for running the test-suite. > > The updated patch adds two changes: > - when the speculative type semantics fail, "dprogress" is also reset to its > original value, so the runDeferredSemantic does not think it should continue. > - when template instantiation fails, it is now also removed from the > module/class member list. DMD 2.032 just removes it from the list of template > instantiations. Now passes the full test suite. This is a candidate for best patch ever. I've never seen anything with the potential to fix so many bugs as once.
Comment #11 by r.sagitario — 2009-10-03T06:27:15Z
(In reply to comment #8) > IMHO this is a VERY promising patch since it potentially fixes many difficult > issues. It's not quite complete though. > I've run it though Walter's test suite, and one test fails (all others pass). > The code below should fail to compile, but instead the compiler gets into an > infinite loop --- it's an ice-on-invalid-code regression. > > ---- > struct A (T) { > mixin B! (T, A! (T)); > } > > A!(int) x; > ---- I've been running into a similar problem with const initializers, that already use a similar mechanism in DMD 2.032. If anonymous classes are involved, it might freeze as well instead of producing an error. It took more time to extract a sensible test case than to patch it, but here it is module test; class C { const int a = (new class Object { int x; }).sizeof + y; S s; } struct S { int s1; } "dmd -c test.d" causes dmd to freeze, instead of reporting "undefined identifer y". And this is the additional patch (line number might be skewed a little): Index: declaration.c =================================================================== --- declaration.c (revision 196) +++ declaration.c (working copy) @@ -1106,6 +1155,7 @@ if (!global.errors && !inferred) { + unsigned progress = Module::dprogress; unsigned errors = global.errors; global.gag++; //printf("+gag\n"); @@ -1127,6 +1177,7 @@ //printf("-gag\n"); if (errors != global.errors) // if errors happened { + Module::dprogress = progress; // we are throwing away the expression copy, so no progress if (global.gag == 0) global.errors = errors; // act as if nothing happened #if DMDV2
Comment #12 by r.sagitario — 2009-12-29T06:52:41Z
Created attachment 537 updated patch again This is another update with the proposed addition and 2 little extensions: - when a struct is searched for identifiers, and its semantic analysis is deferred, try the semantic analysis again. This is already done for classes and is simply extended to structs. - retrying deferred analysis is only done when any "progress" has been made (i.e. new classes or structs have been analysed). This reduces the performance and memory penalties caused by the deferral mechanism. This patch is necessary to compile QtD with DMD 2.037.
Comment #13 by r.sagitario — 2010-03-27T06:29:36Z
As much as I can see, all test cases in this report compile since dmd 1.054 and dmd 2.038.
Comment #14 by clugdbug — 2010-03-27T06:46:57Z
(In reply to comment #13) > As much as I can see, all test cases in this report compile since dmd 1.054 and > dmd 2.038. Confirmed.