[REG2.064] Issue 13221 - [ICE] '0' on line 318 in file 'interpret.c' #3824
Conversation
| @@ -713,8 +713,10 @@ void ctfeCompile(FuncDeclaration *fd) | |||
| */ | |||
| Expression *ctfeInterpret(Expression *e) | |||
| { | |||
| if (e->type == Type::terror) | |||
There was a problem hiding this comment.
Does this change imply expressions may reach ctfeInterpret with an error type that somehow have not been replaced with error expressions? Is this legitimate or should the type check be replaced with an assert?
There was a problem hiding this comment.
With the issue case, e->op was TOKerror but its type was int - I think it should be fixed, but adding assert is too dangerous as a regression fix.
There was a problem hiding this comment.
Please add a comment about this to ctfeInterpret, then.
There was a problem hiding this comment.
Added disabled assert with FIXME comment.
There was a problem hiding this comment.
Ok. Would you consider a second pull request that replaces the check with an assert? Not to be merged into the release of course.
I'm not too worried about documenting it.
There was a problem hiding this comment.
It would need more work to enable the assert. Currently my resource is used for regression issues.
|
I really wish you wouldn't do these refactorings inside pull requests. It makes it much harder to see which lines are important. |
|
OK' I removed some refactoring changes. |
Thankyou. A 10-line diff is really much nicer to read. As the ddmd source is auto-formatted brace-placement fixes are rather pointless. |
|
Auto-merge toggled on |
[REG2.064] Issue 13221 - [ICE] '0' on line 318 in file 'interpret.c'
[REG2.064] Issue 13221 - [ICE] '0' on line 318 in file 'interpret.c'
|
This PR has caused a regression: https://issues.dlang.org/show_bug.cgi?id=13417 |
https://issues.dlang.org/show_bug.cgi?id=13221