summaryrefslogtreecommitdiffstats
path: root/tests
diff options
context:
space:
mode:
authorTim Foley <tfoleyNV@users.noreply.github.com>2020-04-08 15:30:12 -0700
committerGitHub <noreply@github.com>2020-04-08 15:30:12 -0700
commit78acd326a0dd684dd16f1db55bd53937fc38d467 (patch)
tree7e992ed01635256b6280df8c9263032712b87974 /tests
parent6274e175a2b6a07f448feadd4d7da35b2784d746 (diff)
Replace /* unhandled */ in source emit with a real error (#1313)
For a long time the various source-to-source back-ends have been emitted the text `/* unhandled */` when they encounter an IR instruction opcode that didn't have any emit logic implemented. This choice had two apparent benefits: * In most common cases, emitting `/* unhandled */` in place of an expression would lead to downstream compilation failure, so most catastrophic cases seemed to work as desired (e.g., if we emit `int i = /* unhandled */;` we get a downstream parse error and we know something is wrong. * In a few cases, if a dead-but-harmless instruction slips through (e.g., a type formed in the body of a specialized generic function), we would emit `/* unhandled */;`, which is a valid empty statement. It is already clear from the above write-up that the benefits of the policy aren't really that compelling, and where it has recently turned out to be a big liability is that there are actually plenty of cases where emitting `/* unhandled */` instead of a sub-expression won't cause downstream compilation failure, and will instead silently compute incorrect results: * Emitting `/* unhandled */ + b` instead of `a + b` * Emitting `/* unhandled */(a)` instead of `f(a)`, or even `/* unhandled */(a, b)` instead of `f(a,b)` * Emitting `f(/*unhandled*/)` instead of `f(a)` in cases where `f` is a built-in with both zero-argument and one-argument overloads The right fix here is simple: where we would have emitted `/* unhandled */` to the output we should instead diagnose an internal compiler error, thus leading to compilation failure. This change appears to pass all our current tests, but it is possible that there are going to be complicated cases in user code that were relying on the previous lax behavior. I know from experience that we sometimes see `/* unhandled */` in output for generics, and while we have eliminated many of those cases I don't have confidence we've dealt with them all. When this change lands we should make sure that the first release that incorporates it is marked as potentially breaking for clients, and we should make sure to test the changes in the context of the client codebases before those codebases integrate the new release.
Diffstat (limited to 'tests')
0 files changed, 0 insertions, 0 deletions