Bug 4269 – Regression(2.031): invalid type accepted if evaluated while errors are gagged

The D2 code (tried with DMD 2.042 and 2.046): static if(__traits(compiles,A.sizeof)) pragma(msg, "A.sizeof compiles!"); class A { void foo(B b); } compiles without error with "dmd -c test.d" or even links if foo is made final. This is caused by the error when evaluating B is muted while processing __traits(compiles), but A is never revisited later. A debug version of DMD outputs ty = 37, '_error_' assert glue.c(1059) 0 This can happen whenever globals.gag is non-zero, i.e. with speculative semantic analysis.

Generated an error message in 2.030 and earlier.

Here's a test case which incorrectly compiles on D1.055 and later, but generated an error on DMD1.053 and earlier. static if(!is(typeof(A[0]))) pragma(msg, "A.sizeof doesn't compile!"); struct A { B foo(){} } And this variation is ICE(glue.c) on 2.047, but generates wrong code on 2.031-2.046. static if(!is(typeof(A[0]))) pragma(msg, "A.sizeof doesn't compile!"); struct A { void foo(B b){} }

This test case it's a problem with is(), not static if. This behaviour was introduced in 2.047. -------- enum bool WWW = is(typeof(A.x)); class A { B blah; void foo(B b){} } --------

*** Issue 5079 has been marked as a duplicate of this issue. ***

The problem seems to be a huge forward reference bug. When semantic is first called on a declaration while errors are gagged, it is never attempted again. This leads to errors never being printed, and compilation happily continuing to code generation. The solution is probably the same as for bug 4042 - add the ability to rewind the semantic pass on failure to each type of declaration's semantic routines. Some fun test cases: // Compiles without error //static if(is(typeof(X1.init))) {} //class X1 { Y y; } // Compiles without error //static if(is(typeof(X2.init))) {} //struct X2 { Y y; } // Assertion failure: '0' on line 1123 in file 'glue.c' //static if(is(typeof(X3.init))) {} //void X3(T) { } // Compiles without error //static if(is(typeof(X4.init))) {} //Y X4() { return typeof(return).init; } // Access violation in mtype.c:6819 //static if(is(typeof(X5.init))) {} //typedef Y X5; // Compiles without error //static if(is(typeof(X6.init))) {} //alias Y X6; // Assertion failure: '0' on line 1123 in file 'glue.c' //static if(is(typeof(X7))) {} //Y X7; // Compiles without error //static if(is(typeof(X8.init))) {} //class X8 : Y {} // Compiles without error //static if(is(typeof(X9.init))) {} //interface X9 : Y {} // Compiles without error //static if(is(typeof(X9.init))) {} //interface X9 { Y y; } // Doesn't even need the errors gagged - what the hell? // Probably a different bug //struct X10 { alias Y this; } // Assertion failure: '0' on line 1123 in file 'glue.c' //static if(is(typeof(X11.init))) {} //const { Y X11; } // Compiles without error //static if(is(typeof(X12.init))) {} //enum X12 = Y; // Compiles without error //static if(is(typeof(X13!(0).init))) {} //template X13(Y y) {} // Compiles without error //static if(is(typeof(X14.init))) {} //struct X14 { int a; alias a this; alias a this; } // Compiles without error //static if(is(typeof(X15()))) {} //auto X15() { alias x this; } // Compiles without error //static if(is(typeof(X16))) {} //alias X16 X16;

I think there are a couple of different issues here. Some involve forward referenced declarations, as you've said. But some of these occur because they have semantic run while errors are _incorrectly_ gagged. In particular, when semantic3 is run on a function, errors shouldn't be gagged unless the function is part of a template which was speculatively instantiated. I'm halfway through a patch for this.

(In reply to comment #6) > I think there are a couple of different issues here. Some involve forward > referenced declarations, as you've said. > But some of these occur because they have semantic run while errors are > _incorrectly_ gagged. In particular, when semantic3 is run on a function, > errors shouldn't be gagged unless the function is part of a template which was > speculatively instantiated. I'm halfway through a patch for this. Does your gagging solution fix the problem for all semantic passes, or just semantic3? I couldn't think of a way to do it without rewinding semantic on error, that gets error messages appearing in the right places. Since you've got it under control, I'll work on something else. Should I close the pull request, does your patch cover that?

Related, but not the same: bug 6602.

4269 - Regression(2.031): invalid type accepted if evaluated while errors are gagged https://github.com/D-Programming-Language/dmd/commit/106a4c530f1b55c5e7a7f20c841c73fc882c9a6e

That patch only fixes this issue for alias and typedef, not other declarations. D2 commit: https://github.com/D-Programming-Language/dmd/commit/a12dfec8b6427ae08730ff52c6b54e83f16906e3

The whole idea of error gagging is probably bogus because of this. I don't know what to do about it. One possibility might be to attach gagged error messages to the symbol having semantic() run on, and then "replaying" those messages if the symbol is used outside of the gag.

Don seems to have a handle on this. He explained his solution to me in comments on https://github.com/D-Programming-Language/dmd/pull/652 and it seems quite solid.

Commit pushed to master at https://github.com/D-Programming-Language/dmd https://github.com/D-Programming-Language/dmd/commit/75ccf8c3ae057aad7128d710fe735f7772be0471 Issue 4269 - Regression(2.031): invalid type accepted if evaluated while errors are gagged

My patch doesn't thoroughly fix this, it just gets it off the regression list. It outputs a generic error message.

So far my patch fixes the original bug report, and about 60% of Daniel's extra test cases.

Even with the new patch, this example from comment 5 : static if(is(typeof(X3.init))) {} void X3(T3) { } is an ICE(glue.c) for 2.047 on. It generated an error on 2.045 and earlier. (On 2.046 it silently generated bad code). Perhaps it should be treated as a different bug.

(In reply to comment #16) > Even with the new patch, this example from comment 5 : > > static if(is(typeof(X3.init))) {} > void X3(T3) { } > > is an ICE(glue.c) for 2.047 on. It generated an error on 2.045 and earlier. (On > 2.046 it silently generated bad code). Perhaps it should be treated as a > different bug. Yep, different bug. The crash is due to an error in a parameter type not resulting in TypeFunction::semantic returning terror. (Or at least, that and the fact it reaches code generation due to this bug.) I swear I fixed this in a pull request last year, but it must've been part of something that never got accepted.

Having a closer look at Walter's patch, this only fixes the issue for class, struct and interface declarations. From the list in comment 5, function, variable, template and alias this declarations still need to be fixed to get this off the regression list. Don, I thought you meant you'd tested with your new patch and it still crashed - is this the case or is this only with Walter's fix? The underlying reason is the same, but when this bug is fixed the crash will disappear as it will never get to codegen. Walter, please see the list in comment 5, everything here needs to fail before this stops being an ice/regression. Please also note that https://github.com/D-Programming-Language/dmd/commit/75ccf8c3ae057aad7128d710fe735f7772be0471 overwrote three test cases from this bug that dealt with alias and template declarations.

To be more specific - any declaration semantic routine that might result in errors needs to give an error the second time semantic is run too - the reason it happens with the X3 test case is because errors in parameter types are not propagated to the function type, but there are plenty of other errors in FuncDeclaration::semantic that will be silently ignored as the type isn't set to Type::terror. I haven't looked at the other declaration types yet but there are probably cases there too. Some FuncDeclaration cases: //static if (is(typeof(X17.init))) {} //scope void X17() {} //static if (is(typeof(X18.init))) {} //abstract void X18() {} //static if (is(typeof(X19.init))) {} //override void X19() {} //static if (is(typeof(X20.init))) {} //const void X20() {} //static if (is(typeof(X21.init))) {} //immutable void X21() {} //static if (is(typeof(main.init))) {} //string main(); //static if (is(typeof(main.init))) {} //auto main() { return ""; } //static if (is(typeof(main.init))) {} //void main(int) { } Some of these are just accepts-invalid with meaningless attributes, but the 'main' ones will generate wrong code. Errors in the declaration type are not all that needs to be monitored - either the error count needs to be examined or every possible error needs to set the type to Type::terror.

The approach of using Declaration::type to signal semantic failed also relies on Type::semantic returning Type::terror on every error - this is not followed everywhere.

My upcoming patch fixes all of your cases in comment 5 except #13. My comment in comment 16 is about Walter's commit, not my patch. But it's not quite ready, it fails some cases in the test suite related to __traits(compiles) with deprecated and purity. I *think* I just have to do a bit more work on enabling and disabling gagging at the right time.

Commit pushed to master at https://github.com/D-Programming-Language/dmd https://github.com/D-Programming-Language/dmd/commit/ae73e0e9e0cca35cc47ebcb08700f674a0c77ad0 Merge pull request #706 from yebblies/issue4269x Restore tests that were accidentally removed.

Commit pushed to dmd-1.x at https://github.com/D-Programming-Language/dmd https://github.com/D-Programming-Language/dmd/commit/baf9d8c832c50e15912d16c7dd9f85fcf440522f Issue 4269 - Regression(2.031): invalid type accepted if evaluated while errors are gagged

*** Issue 6296 has been marked as a duplicate of this issue. ***

All test cases now correctly give an error, except for case 13, which is fixed in https://github.com/D-Programming-Language/dmd/pull/1370 There are also diagnostic problems with 3 other test cases, I have created bug 9146 for those. It is much less serious than this bug.

Commit pushed to master at https://github.com/D-Programming-Language/dmd https://github.com/D-Programming-Language/dmd/commit/2a1d419a4d87c9a29d3d07f78fc8319ed8a5fc37 Fix bug 4269 case 13:invalid template accepted if evaluated while errors are gagged Need to ungag template declarations if they are forward referenced.

Commit pushed to dmd-1.x at https://github.com/D-Programming-Language/dmd https://github.com/D-Programming-Language/dmd/commit/f0a12c85632e5bba8bf0752aa66f2ab97317e5c9 Fix bug 4269 case 13:invalid template accepted if evaluated while errors are gagged Need to ungag template declarations if they are forward referenced.

Commit pushed to master at https://github.com/D-Programming-Language/dmd https://github.com/D-Programming-Language/dmd/commit/821cd3c604e08dcad7dccca5428389252555ad9c Improve bug4269 test cases