Regularize how we handle "impossible" conditions in internal APIs.

In a switch's default clause that is unexpected, for example, we should
    throw ExceptionUtilities.UnexpectedValue(value);
Long ago this was a standard but we've drifted from it.
Closes #3495

docs/compilers/Design/README.md

@@ -0,0 +1,4 @@
+Design Docs
+===========
+
+This directory is a place for design documents for the Roslyn compilers.

docs/compilers/Design/Unexpected Conditions.md

@@ -0,0 +1,9 @@
+These are [guidelines](https://www.youtube.com/watch?v=6GMkuPiIZ2k) for writing code that may encounter "impossible" and unexpected program conditions. Like other *coding guidelines*, there is value in being somewhat uniform across the code base even though we may have opinions that differ. When you have a good reason to vary from these guidelines, please add comments that explain the variance for those who come after you.
+
+- Use `Debug.Assert(Condition)` to document invariants in the code. Although these dissolve away into nothing in non-Debug builds, as an open-source project we have little control over whether our customers are running our code with Debug enabled. Therefore such assertions should not consume excessive execution time. We may consider having such assertions run in production builds in the future. If we find they are too expensive we may create work items to improve their performance.
+- If you write a switch statement that is intended to be exhaustive of the possibilities, add a default branch with `throw ExceptionUtilities.UnexpectedValue(switchExpression);`.
+- Similarly, if you have a sequence of `if-then-else if` statements that is intended to be exhaustive of the possibilities, add a final "impossible" else branch with `throw ExceptionUtilities.Unreachable;` or, if is convenient to provide interesting, easy to obtain data for the diagnostic, use `ExceptionUtilities.UnexpectedValue`. Do this also for other situations where the code reaches a point that is "impossible".
+- Validation of preconditions in public APIs should be done with plain code. Report a diagnostic if one exists for the situation (e.g. a syntax error), or throw an appropriate exception such as `ArgumentNullException` or `ArgumentException`.
+- If you run into some other weird error case that would be fatal, throw `InvalidOperationException` with interesting, straightforward to get, data in the message. These should be rare and should be accompanied by a comment explaining why an `Assert` is not sufficient.
+
+By following these guidelines, you should find no reason to write `Debug.Assert(false)`. If you find that in your code consider if one of these bullets suggests a different way to handle the situation.

src/Compilers/CSharp/Portable/Binder/BinderFactory.BinderFactoryVisitor.cs

@@ -262,8 +262,7 @@ public override Binder VisitAccessorDeclaration(AccessorDeclarationSyntax parent
                                     break;
                                 }
                             default:
-                                Debug.Assert(false, "Accessor unexpectedly attached to " + propertyOrEventDecl.Kind());
-                                break;
+                                throw ExceptionUtilities.UnexpectedValue(propertyOrEventDecl.Kind());
                         }
 
                         if ((object)accessor != null)

src/Compilers/CSharp/Portable/Binder/Binder_Constraints.cs

@@ -219,8 +219,7 @@ private static bool IsValidConstraintType(TypeConstraintSyntax syntax, TypeSymbo
                 // script class is synthetized, never used as a constraint
 
                 default:
-                    Debug.Assert(false, "Unexpected type kind: " + type.TypeKind);
-                    return false;
+                    throw ExceptionUtilities.UnexpectedValue(type.TypeKind);
             }
 
             if (type.ContainsDynamic())

src/Compilers/CSharp/Portable/Binder/Binder_Conversions.cs

@@ -3,6 +3,7 @@
 using System.Collections.Generic;
 using System.Diagnostics;
 using Microsoft.CodeAnalysis.CSharp.Symbols;
+using Roslyn.Utilities;
 
 namespace Microsoft.CodeAnalysis.CSharp
 {
@@ -890,8 +891,8 @@ private static object DoUncheckedConversion(SpecialType destinationType, Constan
                             case SpecialType.System_Single:
                             case SpecialType.System_Double: return (double)byteValue;
                             case SpecialType.System_Decimal: return (decimal)byteValue;
+                            default: throw ExceptionUtilities.UnexpectedValue(destinationType);
                         }
-                        break;
                     case ConstantValueTypeDiscriminator.Char:
                         char charValue = value.CharValue;
                         switch (destinationType)
@@ -908,8 +909,8 @@ private static object DoUncheckedConversion(SpecialType destinationType, Constan
                             case SpecialType.System_Single:
                             case SpecialType.System_Double: return (double)charValue;
                             case SpecialType.System_Decimal: return (decimal)charValue;
+                            default: throw ExceptionUtilities.UnexpectedValue(destinationType);
                         }
-                        break;
                     case ConstantValueTypeDiscriminator.UInt16:
                         ushort uint16Value = value.UInt16Value;
                         switch (destinationType)
@@ -926,8 +927,8 @@ private static object DoUncheckedConversion(SpecialType destinationType, Constan
                             case SpecialType.System_Single:
                             case SpecialType.System_Double: return (double)uint16Value;
                             case SpecialType.System_Decimal: return (decimal)uint16Value;
+                            default: throw ExceptionUtilities.UnexpectedValue(destinationType);
                         }
-                        break;
                     case ConstantValueTypeDiscriminator.UInt32:
                         uint uint32Value = value.UInt32Value;
                         switch (destinationType)
@@ -944,8 +945,8 @@ private static object DoUncheckedConversion(SpecialType destinationType, Constan
                             case SpecialType.System_Single: return (double)(float)uint32Value;
                             case SpecialType.System_Double: return (double)uint32Value;
                             case SpecialType.System_Decimal: return (decimal)uint32Value;
+                            default: throw ExceptionUtilities.UnexpectedValue(destinationType);
                         }
-                        break;
                     case ConstantValueTypeDiscriminator.UInt64:
                         ulong uint64Value = value.UInt64Value;
                         switch (destinationType)
@@ -962,8 +963,8 @@ private static object DoUncheckedConversion(SpecialType destinationType, Constan
                             case SpecialType.System_Single: return (double)(float)uint64Value;
                             case SpecialType.System_Double: return (double)uint64Value;
                             case SpecialType.System_Decimal: return (decimal)uint64Value;
+                            default: throw ExceptionUtilities.UnexpectedValue(destinationType);
                         }
-                        break;
                     case ConstantValueTypeDiscriminator.SByte:
                         sbyte sbyteValue = value.SByteValue;
                         switch (destinationType)
@@ -980,8 +981,8 @@ private static object DoUncheckedConversion(SpecialType destinationType, Constan
                             case SpecialType.System_Single:
                             case SpecialType.System_Double: return (double)sbyteValue;
                             case SpecialType.System_Decimal: return (decimal)sbyteValue;
+                            default: throw ExceptionUtilities.UnexpectedValue(destinationType);
                         }
-                        break;
                     case ConstantValueTypeDiscriminator.Int16:
                         short int16Value = value.Int16Value;
                         switch (destinationType)
@@ -998,8 +999,8 @@ private static object DoUncheckedConversion(SpecialType destinationType, Constan
                             case SpecialType.System_Single:
                             case SpecialType.System_Double: return (double)int16Value;
                             case SpecialType.System_Decimal: return (decimal)int16Value;
+                            default: throw ExceptionUtilities.UnexpectedValue(destinationType);
                         }
-                        break;
                     case ConstantValueTypeDiscriminator.Int32:
                         int int32Value = value.Int32Value;
                         switch (destinationType)
@@ -1016,8 +1017,8 @@ private static object DoUncheckedConversion(SpecialType destinationType, Constan
                             case SpecialType.System_Single: return (double)(float)int32Value;
                             case SpecialType.System_Double: return (double)int32Value;
                             case SpecialType.System_Decimal: return (decimal)int32Value;
+                            default: throw ExceptionUtilities.UnexpectedValue(destinationType);
                         }
-                        break;
                     case ConstantValueTypeDiscriminator.Int64:
                         long int64Value = value.Int64Value;
                         switch (destinationType)
@@ -1034,8 +1035,8 @@ private static object DoUncheckedConversion(SpecialType destinationType, Constan
                             case SpecialType.System_Single: return (double)(float)int64Value;
                             case SpecialType.System_Double: return (double)int64Value;
                             case SpecialType.System_Decimal: return (decimal)int64Value;
+                            default: throw ExceptionUtilities.UnexpectedValue(destinationType);
                         }
-                        break;
                     case ConstantValueTypeDiscriminator.Single:
                     case ConstantValueTypeDiscriminator.Double:
                         // This code used to invoke CheckConstantBounds and return constant zero if the value is not within the target type.
@@ -1060,8 +1061,8 @@ private static object DoUncheckedConversion(SpecialType destinationType, Constan
                             case SpecialType.System_Single: return (double)(float)doubleValue;
                             case SpecialType.System_Double: return (double)doubleValue;
                             case SpecialType.System_Decimal: return (value.Discriminator == ConstantValueTypeDiscriminator.Single) ? (decimal)(float)doubleValue : (decimal)doubleValue;
+                            default: throw ExceptionUtilities.UnexpectedValue(destinationType);
                         }
-                        break;
                     case ConstantValueTypeDiscriminator.Decimal:
                         decimal decimalValue = CheckConstantBounds(destinationType, value.DecimalValue) ? value.DecimalValue : 0m;
                         switch (destinationType)
@@ -1078,13 +1079,15 @@ private static object DoUncheckedConversion(SpecialType destinationType, Constan
                             case SpecialType.System_Single: return (double)(float)decimalValue;
                             case SpecialType.System_Double: return (double)decimalValue;
                             case SpecialType.System_Decimal: return (decimal)decimalValue;
+                            default: throw ExceptionUtilities.UnexpectedValue(destinationType);
                         }
-                        break;
+                    default:
+                        throw ExceptionUtilities.UnexpectedValue(value.Discriminator);
                 }
             }
 
-            Debug.Assert(false, "Unexpected case in constant folding");
-            return value.Value;
+            // all cases should have been handled in the switch above.
+            // return value.Value;
         }
 
         public static bool CheckConstantBounds(SpecialType destinationType, ConstantValue value)
@@ -1163,10 +1166,10 @@ private static object CanonicalizeConstant(ConstantValue value)
                 case ConstantValueTypeDiscriminator.Single:
                 case ConstantValueTypeDiscriminator.Double: return value.DoubleValue;
                 case ConstantValueTypeDiscriminator.Decimal: return value.DecimalValue;
+                default: throw ExceptionUtilities.UnexpectedValue(value.Discriminator);
             }
 
-            Debug.Assert(false, "unexpected constant in CanonicalizeConstant");
-            return value.Value;
+            // all cases handled in the switch, above.
         }
     }
 }

src/Compilers/CSharp/Portable/Binder/Binder_Crefs.cs

@@ -34,9 +34,7 @@ private ImmutableArray<Symbol> BindCrefInternal(CrefSyntax syntax, out Symbol am
                 case SyntaxKind.ConversionOperatorMemberCref:
                     return BindMemberCref((MemberCrefSyntax)syntax, containerOpt: null, ambiguityWinner: out ambiguityWinner, diagnostics: diagnostics);
                 default:
-                    Debug.Assert(false, "Unexpected cref kind " + syntax.Kind());
-                    ambiguityWinner = null;
-                    return ImmutableArray<Symbol>.Empty;
+                    throw ExceptionUtilities.UnexpectedValue(syntax.Kind());
             }
         }
 
@@ -127,10 +125,7 @@ private ImmutableArray<Symbol> BindMemberCref(MemberCrefSyntax syntax, Namespace
                     result = BindConversionOperatorMemberCref((ConversionOperatorMemberCrefSyntax)syntax, containerOpt, out ambiguityWinner, diagnostics);
                     break;
                 default:
-                    Debug.Assert(false, "Unexpected member cref kind " + syntax.Kind());
-                    ambiguityWinner = null;
-                    result = ImmutableArray<Symbol>.Empty;
-                    break;
+                    throw ExceptionUtilities.UnexpectedValue(syntax.Kind());
             }
 
             if (!result.Any())

src/Compilers/CSharp/Portable/Binder/Binder_Expressions.cs

@@ -1399,11 +1399,7 @@ public BoundExpression BindLabel(ExpressionSyntax node, DiagnosticBag diagnostic
                 return BadExpression(node, result.Kind);
             }
 
-            if (!result.IsSingleViable)
-            {
-                Debug.Assert(false, "If this happens, we need to deal with multiple label definitions.");
-            }
-
+            Debug.Assert(result.IsSingleViable, "If this happens, we need to deal with multiple label definitions.");
             var symbol = (LabelSymbol)result.Symbols.First();
             result.Free();
             return new BoundLabel(node, symbol, null);

src/Compilers/CSharp/Portable/Binder/Binder_InterpolatedString.cs

@@ -87,7 +87,7 @@ private BoundExpression BindInterpolatedString(InterpolatedStringExpressionSynta
                             continue;
                         }
                     default:
-                        throw ExceptionUtilities.Unreachable;
+                        throw ExceptionUtilities.UnexpectedValue(content.Kind());
                 }
             }