Bug 3602 – ICE(tocsym.c) compiling a class, if its super class has preconditions

Status
RESOLVED
Resolution
FIXED
Severity
regression
Priority
P2
Component
dmd
Product
D
Version
D1 (retired)
Platform
All
OS
All
Creation time
2009-12-08T08:44:14Z
Last change time
2017-10-01T20:41:08Z
Keywords
ice-on-valid-code, patch
Assigned to
No Owner
Creator
Qian Xu
Blocks
340

Comments

Comment #0 by qian.xu — 2009-12-08T08:44:14Z
Dear Developers, I see an error message while dmd tries to compile a class: ========= ERROR ========= Box.d(7): Error: function __require forward declaration linkage = 0 dmd: tocsym.c:381: virtual Symbol* FuncDeclaration::toSymbol(): Assertion `0' failed. Aborted ========= ERROR ========= dmd: tocsym.c:381: virtual Symbol* FuncDeclaration::toSymbol(): Assertion `0' failed.
Comment #1 by qian.xu — 2009-12-08T08:49:26Z
Sorry for the first commit. The description is incomplete. Now the part 2: I have two classes as follows: ====== FILE: Box.d ====== module Box; class Box { void paint(int x, int y) in { assert(x > 0); assert(y > 0); } body { } } ====== FILE: Box.d ====== ====== FILE: ImageBox.d ====== module ImageBox; class ImageBox: Box { void override paint(int x, int y) in { assert(x > 0); assert(y > 0); } body { } } ====== FILE: ImageBox.d ====== I compile the Box.d using "dmd -w -debug -inline -version=Posix -version=Tango Box.d -c", it works. And then I compile the ImageBox.d using "dmd -w -debug -inline -version=Posix -version=Tango ImageBox.d -c", the dmd compile returns an error. ========= ERROR ========= Box.d(7): Error: function __require forward declaration linkage = 0 dmd: tocsym.c:381: virtual Symbol* FuncDeclaration::toSymbol(): Assertion `0' failed. Aborted ========= ERROR ========= Note that if I write "private" before this method paint, it works. I think this issue is pretty critical, now I have to remove all preconditions in my super classes.
Comment #2 by clugdbug — 2010-01-27T00:46:38Z
This is crashing because the contracts are implemented as functions fdrequire, fdensure, which are NESTED functions of the function ('paint()') being called. fdrequire->semantic() and fdensure->semantic() need to be run before code generation of the overridden function. Because they're nested functions, fdrequire->semantic() is called when running semantic3() on 'box.paint()'. But, if because box.paint() was only imported, box.paint()->semantic3() never gets run! To fix the bug, we need to make sure that fdrequire->semantic() does get called sometime before code generation. I've tried running it immediately after fdrequire is created; that allows the test case to compile when the two modules are compiled separately, but it causes a different ICE when they are compiled together. I also tried running semantic() from inside mergeRequire(), but I wasn't successful.
Comment #3 by robert — 2010-04-03T05:18:21Z
*** Issue 4055 has been marked as a duplicate of this issue. ***
Comment #4 by smjg — 2010-06-14T18:30:05Z
The given testcase is invalid. However, by fixing the errors, it's reproducible under Windows: ----- imagebox.d ----- module imagebox; import box; class ImageBox: Box { override void paint(int x, int y) in { assert(x > 0); assert(y > 0); } body { } } ----- box.d ----- module box; class Box { void paint(int x, int y) in { assert(x > 0); assert(y > 0); } body { } } ---------- C:\Users\Stewart\Documents\Programming\D\Tests\bugs\bz3602>dmd -c imagebox.d box.d(5): Error: function __require forward declaration linkage = 0 Assertion failure: '0' on line 381 in file 'tocsym.c' abnormal program termination ---------- This has broken SDWF.
Comment #5 by clugdbug — 2010-06-15T00:00:53Z
(In reply to comment #4) > The given testcase is invalid. However, by fixing the errors, it's > reproducible under Windows. ?? I had no trouble reproducing it under Windows. Until around DMD 1.050, it used to compile, but that was only because contract inheritance was silently ignored. This code has never worked properly. OTOH I agree that it has the importance of a regression.
Comment #6 by smjg — 2010-06-15T01:39:17Z
(In reply to comment #5) > (In reply to comment #4) > > The given testcase is invalid. However, by fixing the errors, it's > > reproducible under Windows. > > ?? I had no trouble reproducing it under Windows. > Until around DMD 1.050, it used to compile, but that was only because contract > inheritance was silently ignored. This code has never worked properly. OTOH I > agree that it has the importance of a regression. That's very weird. How did it use to handle the fact that Box is an undefined symbol in ImageBox.d?
Comment #7 by bearophile_hugs — 2010-06-15T02:47:30Z
I have not seen the ICE but this asserts (v2.047 with warnings): class A { void foo(int x) in { assert(x > 0); // asserts } body {} } class B : A { override void foo(int y) {} } void main() { auto b = new B; b.foo(10); }
Comment #8 by smjg — 2010-06-15T02:52:55Z
(In reply to comment #7) > assert(x > 0); // asserts Could hardly state the obvious more - unless there was something else you meant to say....
Comment #9 by clugdbug — 2010-09-15T05:15:58Z
PATCH: func.c, line 1658, FuncDeclaration::mergeFrequire() ---------------- for (int i = 0; i < foverrides.dim; i++) { FuncDeclaration *fdv = (FuncDeclaration *)foverrides.data[i]; + /* The semantic pass on the contracts of the overridden functions must + * be completed before code generation occurs (bug 3602). + */ + if (fdv->fdrequire && fdv->fdrequire->semanticRun != PASSsemantic3done) + { + assert(fdv->scope); + fdv->semantic3(fdv->scope); + } sf = fdv->mergeFrequire(sf); if (fdv->fdrequire) { //printf("fdv->frequire: %s\n", fdv->frequire->toChars());
Comment #10 by bugzilla — 2010-09-22T06:24:00Z
Comment #11 by github-bugzilla — 2017-09-25T21:23:15Z
Commit pushed to stable at https://github.com/dlang/dmd https://github.com/dlang/dmd/commit/b79c250bc27659274aa3a93473cd8f550738e3ad Handle bug 3602 and 5230, add tests for them.
Comment #12 by github-bugzilla — 2017-10-01T20:41:08Z
Commit pushed to master at https://github.com/dlang/dmd https://github.com/dlang/dmd/commit/b79c250bc27659274aa3a93473cd8f550738e3ad Handle bug 3602 and 5230, add tests for them.