Minor cleanup of type system code base

- Improve nested types check by switching to a named constant.
- Add assert so that we don't end up recalculating field flags again and again
- Move signature variable hash code generation to TypeHashingAlgorithms
- Fix casing for method hash code generation in TypeHashingAlgorithms
- Change parameter names in TypeHashingAlgorithms to use casing consistent with the method names

9 files changed, 49 insertions, 27 deletions

diff --git a/src/Common/src/TypeSystem/Common/InstantiatedMethod.cs b/src/Common/src/TypeSystem/Common/InstantiatedMethod.cs
index a2848154a..9cf7b929f 100644
--- a/src/Common/src/TypeSystem/Common/InstantiatedMethod.cs
+++ b/src/Common/src/TypeSystem/Common/InstantiatedMethod.cs
@@ -26,7 +26,7 @@ namespace Internal.TypeSystem
 
         protected override int ComputeHashCode()
         {
-            return TypeHashingAlgorithms.ComputeMethodHashcode(OwningType.GetHashCode(), Instantiation.ComputeGenericInstanceHashCode(TypeHashingAlgorithms.ComputeNameHashCode(Name)));
+            return TypeHashingAlgorithms.ComputeMethodHashCode(OwningType.GetHashCode(), Instantiation.ComputeGenericInstanceHashCode(TypeHashingAlgorithms.ComputeNameHashCode(Name)));
         }
 
         public override TypeSystemContext Context
diff --git a/src/Common/src/TypeSystem/Common/MethodDesc.cs b/src/Common/src/TypeSystem/Common/MethodDesc.cs
index 132250561..a3f4ab83e 100644
--- a/src/Common/src/TypeSystem/Common/MethodDesc.cs
+++ b/src/Common/src/TypeSystem/Common/MethodDesc.cs
@@ -223,7 +223,7 @@ namespace Internal.TypeSystem
         /// </summary>
         protected virtual int ComputeHashCode()
         {
-            return TypeHashingAlgorithms.ComputeMethodHashcode(OwningType.GetHashCode(), TypeHashingAlgorithms.ComputeNameHashCode(Name));
+            return TypeHashingAlgorithms.ComputeMethodHashCode(OwningType.GetHashCode(), TypeHashingAlgorithms.ComputeNameHashCode(Name));
         }
 
         public override bool Equals(Object o)
diff --git a/src/Common/src/TypeSystem/Common/SignatureVariable.cs b/src/Common/src/TypeSystem/Common/SignatureVariable.cs
index 2ff9fa215..034a79534 100644
--- a/src/Common/src/TypeSystem/Common/SignatureVariable.cs
+++ b/src/Common/src/TypeSystem/Common/SignatureVariable.cs
@@ -2,6 +2,7 @@
 // Licensed under the MIT license. See LICENSE file in the project root for full license information.
 
 using System;
+using Internal.NativeFormat;
 
 namespace Internal.TypeSystem
 {
@@ -54,7 +55,7 @@ namespace Internal.TypeSystem
 
         public override int GetHashCode()
         {
-            return Index * 0x5498341 + 0x832424;
+            return TypeHashingAlgorithms.ComputeSignatureVariableHashCode(Index, false);
         }
 
         protected override TypeFlags ComputeTypeFlags(TypeFlags mask)
@@ -84,7 +85,7 @@ namespace Internal.TypeSystem
 
         public override int GetHashCode()
         {
-            return Index * 0x7822381 + 0x54872645;
+            return TypeHashingAlgorithms.ComputeSignatureVariableHashCode(Index, true);
         }
 
         protected override TypeFlags ComputeTypeFlags(TypeFlags mask)
diff --git a/src/Common/src/TypeSystem/Common/TypeHashingAlgorithms.cs b/src/Common/src/TypeSystem/Common/TypeHashingAlgorithms.cs
index 5ecd4369e..3a5c7bda5 100644
--- a/src/Common/src/TypeSystem/Common/TypeHashingAlgorithms.cs
+++ b/src/Common/src/TypeSystem/Common/TypeHashingAlgorithms.cs
@@ -67,7 +67,7 @@ namespace Internal.NativeFormat
             return hash1 ^ hash2;
         }
 
-        public static int ComputeArrayTypeHashCode(int elementTypeHashcode, int rank)
+        public static int ComputeArrayTypeHashCode(int elementTypeHashCode, int rank)
         {
             // Arrays are treated as generic types in some parts of our system. The array hashcodes are 
             // carefully crafted to be the same as the hashcodes of their implementation generic types.
@@ -83,7 +83,7 @@ namespace Internal.NativeFormat
                 hashCode = ComputeNameHashCode("System.MDArrayRank" + rank.ToString() + "`1");
             }
 
-            hashCode = (hashCode + _rotl(hashCode, 13)) ^ elementTypeHashcode;
+            hashCode = (hashCode + _rotl(hashCode, 13)) ^ elementTypeHashCode;
             return (hashCode + _rotl(hashCode, 15));
         }
 
@@ -93,9 +93,9 @@ namespace Internal.NativeFormat
         }
 
 
-        public static int ComputePointerTypeHashCode(int pointeeTypeHashcode)
+        public static int ComputePointerTypeHashCode(int pointeeTypeHashCode)
         {
-            return (pointeeTypeHashcode + _rotl(pointeeTypeHashcode, 5)) ^ 0x12D0;
+            return (pointeeTypeHashCode + _rotl(pointeeTypeHashCode, 5)) ^ 0x12D0;
         }
 
         public static int ComputePointerTypeHashCode<T>(T pointeeType)
@@ -104,9 +104,9 @@ namespace Internal.NativeFormat
         }
 
 
-        public static int ComputeByrefTypeHashCode(int parameterTypeHashcode)
+        public static int ComputeByrefTypeHashCode(int parameterTypeHashCode)
         {
-            return (parameterTypeHashcode + _rotl(parameterTypeHashcode, 7)) ^ 0x4C85;
+            return (parameterTypeHashCode + _rotl(parameterTypeHashCode, 7)) ^ 0x4C85;
         }
 
         public static int ComputeByrefTypeHashCode<T>(T parameterType)
@@ -115,9 +115,9 @@ namespace Internal.NativeFormat
         }
 
 
-        public static int ComputeNestedTypeHashCode(int enclosingTypeHashcode, int nestedTypeNameHash)
+        public static int ComputeNestedTypeHashCode(int enclosingTypeHashCode, int nestedTypeNameHash)
         {
-            return (enclosingTypeHashcode + _rotl(enclosingTypeHashcode, 11)) ^ nestedTypeNameHash;
+            return (enclosingTypeHashCode + _rotl(enclosingTypeHashCode, 11)) ^ nestedTypeNameHash;
         }
 
 
@@ -135,14 +135,31 @@ namespace Internal.NativeFormat
         /// <summary>
         /// Produce a hashcode for a specific method
         /// </summary>
-        /// <param name="typeHashcode">Hashcode of the type that owns the method</param>
-        /// <param name="nameOrNameAndGenericArgumentsHashcode">Hashcode of either the name of the method (for non-generic methods) or the GenericInstanceHashCode of the name+generic arguments of the method.</param>
+        /// <param name="typeHashCode">HashCode of the type that owns the method</param>
+        /// <param name="nameOrNameAndGenericArgumentsHashCode">HashCode of either the name of the method (for non-generic methods) or the GenericInstanceHashCode of the name+generic arguments of the method.</param>
         /// <returns></returns>
-        public static int ComputeMethodHashcode(int typeHashcode, int nameOrNameAndGenericArgumentsHashcode)
+        public static int ComputeMethodHashCode(int typeHashCode, int nameOrNameAndGenericArgumentsHashCode)
         {
             // TODO! This hash combining function isn't good, but it matches logic used in the past
             // consider changing to a better combining function once all uses use this function
-            return typeHashcode ^ nameOrNameAndGenericArgumentsHashcode;
+            return typeHashCode ^ nameOrNameAndGenericArgumentsHashCode;
+        }
+
+        /// <summary>
+        /// Produce a hashcode for a generic signature variable
+        /// </summary>
+        /// <param name="index">zero based index</param>
+        /// <param name="method">true if the signature variable describes a method</param>
+        public static int ComputeSignatureVariableHashCode(int index, bool method)
+        {
+            if (method)
+            {
+                return index * 0x7822381 + 0x54872645;
+            }
+            else
+            {
+                return index * 0x5498341 + 0x832424;
+            }
         }
     }
 }
diff --git a/src/Common/src/TypeSystem/Common/TypeSystemContext.cs b/src/Common/src/TypeSystem/Common/TypeSystemContext.cs
index 9e6b2d21d..cd7444ae6 100644
--- a/src/Common/src/TypeSystem/Common/TypeSystemContext.cs
+++ b/src/Common/src/TypeSystem/Common/TypeSystemContext.cs
@@ -333,7 +333,7 @@ namespace Internal.TypeSystem
             {
                 _methodDef = methodDef;
                 _instantiation = instantiation;
-                _hashcode = TypeHashingAlgorithms.ComputeMethodHashcode(methodDef.OwningType.GetHashCode(),
+                _hashcode = TypeHashingAlgorithms.ComputeMethodHashCode(methodDef.OwningType.GetHashCode(),
                     instantiation.ComputeGenericInstanceHashCode(TypeHashingAlgorithms.ComputeNameHashCode(methodDef.Name)));
             }
 
@@ -435,7 +435,7 @@ namespace Internal.TypeSystem
             {
                 _typicalMethodDef = typicalMethodDef;
                 _instantiatedType = instantiatedType;
-                _hashcode = TypeHashingAlgorithms.ComputeMethodHashcode(instantiatedType.GetHashCode(), TypeHashingAlgorithms.ComputeNameHashCode(typicalMethodDef.Name));
+                _hashcode = TypeHashingAlgorithms.ComputeMethodHashCode(instantiatedType.GetHashCode(), TypeHashingAlgorithms.ComputeNameHashCode(typicalMethodDef.Name));
             }
 
             public MethodDesc TypicalMethodDef
diff --git a/src/Common/src/TypeSystem/Ecma/EcmaField.cs b/src/Common/src/TypeSystem/Ecma/EcmaField.cs
index 24836caa9..0e339dc9a 100644
--- a/src/Common/src/TypeSystem/Ecma/EcmaField.cs
+++ b/src/Common/src/TypeSystem/Ecma/EcmaField.cs
@@ -167,6 +167,7 @@ namespace Internal.TypeSystem.Ecma
 
             _fieldFlags.AddFlags(flags);
 
+            Debug.Assert((flags & mask) != 0);
             return flags & mask;
         }
 
diff --git a/src/Common/src/TypeSystem/Ecma/EcmaGenericParameter.cs b/src/Common/src/TypeSystem/Ecma/EcmaGenericParameter.cs
index 5f76a0af7..040611a38 100644
--- a/src/Common/src/TypeSystem/Ecma/EcmaGenericParameter.cs
+++ b/src/Common/src/TypeSystem/Ecma/EcmaGenericParameter.cs
@@ -7,6 +7,7 @@ using System.Reflection.Metadata;
 using System.Threading;
 
 using Internal.TypeSystem;
+using Internal.NativeFormat;
 
 using Debug = System.Diagnostics.Debug;
 
@@ -28,14 +29,7 @@ namespace Internal.TypeSystem.Ecma
             // TODO: Determine what a the right hash function should be. Use stable hashcode based on the type name?
             // For now, use the same hash as a SignatureVariable type.
             GenericParameter parameter = _module.MetadataReader.GetGenericParameter(_handle);
-            if (parameter.Parent.Kind == HandleKind.MethodDefinition)
-            {
-                return parameter.Index * 0x7822381 + 0x54872645;
-            }
-            else
-            {
-                return parameter.Index * 0x5498341 + 0x832424;
-            }
+            return TypeHashingAlgorithms.ComputeSignatureVariableHashCode(parameter.Index, parameter.Parent.Kind == HandleKind.MethodDefinition);
         }
 
         public override TypeSystemContext Context
@@ -54,6 +48,7 @@ namespace Internal.TypeSystem.Ecma
 
             flags |= TypeFlags.GenericParameter;
 
+            Debug.Assert((flags & mask) != 0);
             return flags;
         }
 
diff --git a/src/Common/src/TypeSystem/Ecma/EcmaType.cs b/src/Common/src/TypeSystem/Ecma/EcmaType.cs
index 7b21863c0..4ee82c16d 100644
--- a/src/Common/src/TypeSystem/Ecma/EcmaType.cs
+++ b/src/Common/src/TypeSystem/Ecma/EcmaType.cs
@@ -214,6 +214,7 @@ namespace Internal.TypeSystem.Ecma
                 // All other cases are handled during TypeSystemContext intitialization
             }
 
+            Debug.Assert((flags & mask) != 0);
             return flags;
         }
 
diff --git a/src/Common/src/TypeSystem/Ecma/MetadataExtensions.cs b/src/Common/src/TypeSystem/Ecma/MetadataExtensions.cs
index 0a10b9bb1..0b6ced563 100644
--- a/src/Common/src/TypeSystem/Ecma/MetadataExtensions.cs
+++ b/src/Common/src/TypeSystem/Ecma/MetadataExtensions.cs
@@ -103,9 +103,16 @@ namespace Internal.TypeSystem.Ecma
             }
         }
 
+        // This mask is the fastest way to check if a type is nested from its flags,
+        // but it should not be added to the BCL enum as its semantics can be misleading.
+        // Consider, for example, that (NestedFamANDAssem & NestedMask) == NestedFamORAssem.
+        // Only comparison of the masked value to 0 is meaningful, which is different from
+        // the other masks in the enum.
+        private const TypeAttributes NestedMask = (TypeAttributes)0x00000006;
+
         private static bool IsNested(TypeAttributes flags)
         {
-            return (flags & ((TypeAttributes)0x00000006)) != 0;
+            return (flags & NestedMask) != 0;
         }
     }
 }