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

D issues are now tracked on GitHub. This Bugzilla instance remains as a read-only archive.
Issue 4269 - Regression(2.031): invalid type accepted if evaluated while errors are gagged
Summary: Regression(2.031): invalid type accepted if evaluated while errors are gagged
Status: RESOLVED FIXED
Alias: None
Product: D
Classification: Unclassified
Component: dmd (show other issues)
Version: D2
Hardware: All All
: P2 normal
Assignee: Don
URL:
Keywords: accepts-invalid, ice, pull, wrong-code
: 5079 6296 (view as issue list)
Depends on:
Blocks:
Reported: 2010年06月04日 05:58 UTC by Rainer Schuetze
Modified: 2015年06月09日 05:12 UTC (History)
6 users (show)

See Also:


Attachments
Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this issue.
Description Rainer Schuetze 2010年06月04日 05:58:53 UTC
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.
Comment 1 Don 2010年06月15日 07:25:53 UTC
Generated an error message in 2.030 and earlier.
Comment 2 Don 2010年06月15日 13:41:05 UTC
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){} 
}
Comment 3 Don 2010年08月25日 12:44:15 UTC
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){} 
}
--------
Comment 4 Don 2011年02月11日 05:23:49 UTC
*** Issue 5079 has been marked as a duplicate of this issue. ***
Comment 5 yebblies 2011年07月10日 07:33:53 UTC
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;
Comment 6 Don 2011年08月30日 01:42:25 UTC
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.
Comment 7 yebblies 2011年08月30日 07:58:01 UTC
(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?
Comment 8 David Nadlinger 2011年09月04日 22:51:20 UTC
Related, but not the same: bug 6602.
Comment 9 Walter Bright 2011年09月20日 18:51:40 UTC
4269 - Regression(2.031): invalid type accepted if evaluated while errors are gagged
https://github.com/D-Programming-Language/dmd/commit/106a4c530f1b55c5e7a7f20c841c73fc882c9a6e 
Comment 10 yebblies 2011年09月20日 19:13:36 UTC
That patch only fixes this issue for alias and typedef, not other declarations.
D2 commit: https://github.com/D-Programming-Language/dmd/commit/a12dfec8b6427ae08730ff52c6b54e83f16906e3 
Comment 11 Walter Bright 2012年02月06日 20:59:24 UTC
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.
Comment 12 yebblies 2012年02月06日 22:28:39 UTC
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.
Comment 13 github-bugzilla 2012年02月06日 22:42:30 UTC
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
Comment 14 Walter Bright 2012年02月06日 22:44:36 UTC
My patch doesn't thoroughly fix this, it just gets it off the regression list. It outputs a generic error message.
Comment 15 Don 2012年02月07日 01:05:27 UTC
So far my patch fixes the original bug report, and about 60% of Daniel's extra test cases.
Comment 16 Don 2012年02月09日 07:55:58 UTC
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.
Comment 17 yebblies 2012年02月09日 08:08:33 UTC
(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.
Comment 18 yebblies 2012年02月10日 03:17:54 UTC
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.
Comment 19 yebblies 2012年02月10日 04:05:52 UTC
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.
Comment 20 yebblies 2012年02月10日 04:11:42 UTC
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.
Comment 21 Don 2012年02月10日 07:12:18 UTC
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.
Comment 22 github-bugzilla 2012年02月11日 10:26:23 UTC
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.
Comment 23 github-bugzilla 2012年02月22日 02:22:28 UTC
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
Comment 24 Walter Bright 2012年02月22日 02:25:38 UTC
https://github.com/D-Programming-Language/dmd/pull/729 
Comment 25 Don 2012年05月03日 02:53:14 UTC
*** Issue 6296 has been marked as a duplicate of this issue. ***
Comment 26 Don 2012年12月13日 01:05:42 UTC
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.
Comment 27 github-bugzilla 2012年12月13日 06:46:13 UTC
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.
Comment 28 github-bugzilla 2013年01月16日 01:52:33 UTC
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.
Comment 29 github-bugzilla 2013年08月28日 14:34:46 UTC
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


AltStyle によって変換されたページ (->オリジナル) /