Refactor Microsoft.CSharp's SubstContext (dotnet/corefx#26489)
authorJon Hanna <jon@hackcraft.net>
Mon, 29 Jan 2018 03:45:05 +0000 (03:45 +0000)
committerVladimir Sadov <vsadov@microsoft.com>
Mon, 29 Jan 2018 03:45:05 +0000 (19:45 -0800)
* Remove SubstTypeFlags.NoRefOutDifference

Looked for, but never set.

* Remove SubstTypeFlags.DenormClass

Looked for, but never set.

* Remove SubstTypeFlags.Norm*

Looked for, but never set.

* Remove _stvcClass

No longer used.

* Move init into ctor and make fields readonly

* Remove unused ctors

* Remove counts from SubsContext

Just get them from the arrays.

* FNop() method to IsNop property

* Rename context's arrays

Non-Hungarian based.

* Remove SubstTypeFlags

Only two values in use, so use boolean.

* Remove denormMeth argument to SubstEqualTypeArrays

Always true

* Remove most uses of context in ErrAppendType

Instead of setting to null, just use null.

* Remove paths in SubstType for null typeSrc

Never happens. (And if it did, it would safely have the same result).

Commit migrated from https://github.com/dotnet/corefx/commit/275b172426b42c152ee359601197aeb64f3e8b28

src/libraries/Microsoft.CSharp/src/Microsoft/CSharp/RuntimeBinder/Errors/UserStringBuilder.cs
src/libraries/Microsoft.CSharp/src/Microsoft/CSharp/RuntimeBinder/Semantics/PredefinedMembers.cs
src/libraries/Microsoft.CSharp/src/Microsoft/CSharp/RuntimeBinder/Semantics/SubstitutionContext.cs
src/libraries/Microsoft.CSharp/src/Microsoft/CSharp/RuntimeBinder/Semantics/Types/TypeManager.cs

index 6880a2a..c101aac 100644 (file)
@@ -141,7 +141,7 @@ namespace Microsoft.CSharp.RuntimeBinder.Errors
                 return;
             }
 
-            if (pctx != null && !pctx.FNop() && parent is AggregateSymbol agg && 0 != agg.GetTypeVarsAll().Count)
+            if (pctx != null && !pctx.IsNop && parent is AggregateSymbol agg && 0 != agg.GetTypeVarsAll().Count)
             {
                 CType pType = GetTypeManager().SubstType(agg.getThisType(), pctx);
                 ErrAppendType(pType, null);
@@ -406,21 +406,11 @@ namespace Microsoft.CSharp.RuntimeBinder.Errors
             }
         }
 
-        private void ErrAppendType(CType pType, SubstContext pCtx)
+        private void ErrAppendType(CType pType, SubstContext pctx)
         {
-            ErrAppendType(pType, pCtx, true);
-        }
-
-        private void ErrAppendType(CType pType, SubstContext pctx, bool fArgs)
-        {
-            if (pctx != null)
+            if (pctx != null && !pctx.IsNop)
             {
-                if (!pctx.FNop())
-                {
-                    pType = GetTypeManager().SubstType(pType, pctx);
-                }
-                // We shouldn't use the SubstContext again so set it to NULL.
-                pctx = null;
+                pType = GetTypeManager().SubstType(pType, pctx);
             }
 
             switch (pType.TypeKind)
@@ -441,19 +431,19 @@ namespace Microsoft.CSharp.RuntimeBinder.Errors
                         {
                             if (pAggType.OuterType != null)
                             {
-                                ErrAppendType(pAggType.OuterType, pctx);
+                                ErrAppendType(pAggType.OuterType, null);
                                 ErrAppendChar('.');
                             }
                             else
                             {
                                 // In a namespace.
-                                ErrAppendParentSym(pAggType.OwningAggregate, pctx);
+                                ErrAppendParentSym(pAggType.OwningAggregate, null);
                             }
 
                             ErrAppendName(pAggType.OwningAggregate.name);
                         }
 
-                        ErrAppendTypeParameters(pAggType.TypeArgsThis, pctx, true);
+                        ErrAppendTypeParameters(pAggType.TypeArgsThis, null, true);
                         break;
                     }
 
@@ -494,13 +484,9 @@ namespace Microsoft.CSharp.RuntimeBinder.Errors
                     {
                         CType elementType = ((ArrayType)pType).BaseElementType;
 
-                        if (null == elementType)
-                        {
-                            Debug.Assert(false, "No element type");
-                            break;
-                        }
+                        Debug.Assert(elementType != null, "No element type");
 
-                        ErrAppendType(elementType, pctx);
+                        ErrAppendType(elementType, null);
 
                         for (elementType = pType;
                                 elementType is ArrayType arrType;
@@ -542,12 +528,12 @@ namespace Microsoft.CSharp.RuntimeBinder.Errors
                     ErrAppendString(mod.IsOut ? "out " : "ref ");
 
                     // add base type name
-                    ErrAppendType(mod.ParameterType, pctx);
+                    ErrAppendType(mod.ParameterType, null);
                     break;
 
                 case TypeKind.TK_PointerType:
                     // Generate the base type.
-                    ErrAppendType(((PointerType)pType).ReferentType, pctx);
+                    ErrAppendType(((PointerType)pType).ReferentType, null);
                     {
                         // add the trailing *
                         ErrAppendChar('*');
@@ -555,7 +541,7 @@ namespace Microsoft.CSharp.RuntimeBinder.Errors
                     break;
 
                 case TypeKind.TK_NullableType:
-                    ErrAppendType(((NullableType)pType).UnderlyingType, pctx);
+                    ErrAppendType(((NullableType)pType).UnderlyingType, null);
                     ErrAppendChar('?');
                     break;
 
index 0a12b03..958f487 100644 (file)
@@ -406,9 +406,8 @@ namespace Microsoft.CSharp.RuntimeBinder.Semantics
                         methsym.isStatic == isStatic &&
                         methsym.isVirtual == isVirtual &&
                         methsym.typeVars.Count == cMethodTyVars &&
-                        GetTypeManager().SubstEqualTypes(methsym.RetType, returnType, null, methsym.typeVars, SubstTypeFlags.DenormMeth) &&
-                        GetTypeManager().SubstEqualTypeArrays(methsym.Params, argumentTypes, (TypeArray)null,
-                            methsym.typeVars, SubstTypeFlags.DenormMeth))
+                        GetTypeManager().SubstEqualTypes(methsym.RetType, returnType, null, methsym.typeVars, true) &&
+                        GetTypeManager().SubstEqualTypeArrays(methsym.Params, argumentTypes, null, methsym.typeVars))
                     {
                         return methsym;
                     }
index 859f5b5..6422e95 100644 (file)
@@ -6,95 +6,36 @@ using System;
 
 namespace Microsoft.CSharp.RuntimeBinder.Semantics
 {
-    // Used to specify whether and which type variables should be normalized.
-    [Flags]
-    internal enum SubstTypeFlags
-    {
-        NormNone = 0x00,
-        NormClass = 0x01,  // Replace class type variables with the normalized (standard) ones.
-        NormMeth = 0x02,   // Replace method type variables with the normalized (standard) ones.
-        NormAll = NormClass | NormMeth,
-        DenormClass = 0x04,  // Replace normalized (standard) class type variables with the given class type args.
-        DenormMeth = 0x08,   // Replace normalized (standard) method type variables with the given method type args.
-        DenormAll = DenormClass | DenormMeth,
-        NoRefOutDifference = 0x10
-    }
-
     internal sealed class SubstContext
     {
-        public CType[] prgtypeCls;
-        public int ctypeCls;
-        public CType[] prgtypeMeth;
-        public int ctypeMeth;
-        public SubstTypeFlags grfst;
+        public readonly CType[] ClassTypes;
+        public readonly CType[] MethodTypes;
+        public readonly bool DenormMeth;
 
-        public SubstContext(TypeArray typeArgsCls, TypeArray typeArgsMeth, SubstTypeFlags grfst)
+        public SubstContext(TypeArray typeArgsCls, TypeArray typeArgsMeth, bool denormMeth)
         {
-            Init(typeArgsCls, typeArgsMeth, grfst);
+            typeArgsCls?.AssertValid();
+            ClassTypes = typeArgsCls?.Items ?? Array.Empty<CType>();
+            typeArgsMeth?.AssertValid();
+            MethodTypes = typeArgsMeth?.Items ?? Array.Empty<CType>();
+            DenormMeth = denormMeth;
         }
 
         public SubstContext(AggregateType type)
-            : this(type, null, SubstTypeFlags.NormNone)
+            : this(type, null, false)
         {
         }
 
         public SubstContext(AggregateType type, TypeArray typeArgsMeth)
-            : this(type, typeArgsMeth, SubstTypeFlags.NormNone)
-        {
-        }
-
-        private SubstContext(AggregateType type, TypeArray typeArgsMeth, SubstTypeFlags grfst)
-        {
-            Init(type?.TypeArgsAll, typeArgsMeth, grfst);
-        }
-
-        public SubstContext(CType[] prgtypeCls, int ctypeCls, CType[] prgtypeMeth, int ctypeMeth)
-            : this(prgtypeCls, ctypeCls, prgtypeMeth, ctypeMeth, SubstTypeFlags.NormNone)
-        {
-        }
-
-        private SubstContext(CType[] prgtypeCls, int ctypeCls, CType[] prgtypeMeth, int ctypeMeth, SubstTypeFlags grfst)
+            : this(type, typeArgsMeth, false)
         {
-            this.prgtypeCls = prgtypeCls;
-            this.ctypeCls = ctypeCls;
-            this.prgtypeMeth = prgtypeMeth;
-            this.ctypeMeth = ctypeMeth;
-            this.grfst = grfst;
         }
 
-        public bool FNop()
+        private SubstContext(AggregateType type, TypeArray typeArgsMeth, bool denormMeth)
+            : this(type?.TypeArgsAll, typeArgsMeth, denormMeth)
         {
-            return 0 == ctypeCls && 0 == ctypeMeth && 0 == (grfst & SubstTypeFlags.NormAll);
         }
 
-        // Initializes a substitution context. Returns false iff no substitutions will ever be performed.
-        private void Init(TypeArray typeArgsCls, TypeArray typeArgsMeth, SubstTypeFlags grfst)
-        {
-            if (typeArgsCls != null)
-            {
-                typeArgsCls.AssertValid();
-                ctypeCls = typeArgsCls.Count;
-                prgtypeCls = typeArgsCls.Items;
-            }
-            else
-            {
-                ctypeCls = 0;
-                prgtypeCls = null;
-            }
-
-            if (typeArgsMeth != null)
-            {
-                typeArgsMeth.AssertValid();
-                ctypeMeth = typeArgsMeth.Count;
-                prgtypeMeth = typeArgsMeth.Items;
-            }
-            else
-            {
-                ctypeMeth = 0;
-                prgtypeMeth = null;
-            }
-
-            this.grfst = grfst;
-        }
+        public bool IsNop => ClassTypes.Length == 0 & MethodTypes.Length == 0;
     }
 }
index 4842d34..dbaf314 100644 (file)
@@ -21,14 +21,12 @@ namespace Microsoft.CSharp.RuntimeBinder.Semantics
         private SymbolTable _symbolTable;
 
         private readonly StdTypeVarColl _stvcMethod;
-        private readonly StdTypeVarColl _stvcClass;
 
         public TypeManager(BSYMMGR bsymmgr, PredefinedTypes predefTypes)
         {
             _typeTable = new TypeTable();
 
             _stvcMethod = new StdTypeVarColl();
-            _stvcClass = new StdTypeVarColl();
             _BSymmgr = bsymmgr;
             _predefTypes = predefTypes;
         }
@@ -202,37 +200,27 @@ namespace Microsoft.CSharp.RuntimeBinder.Semantics
 
         public AggregateSymbol GetNullable() => GetPredefAgg(PredefinedType.PT_G_OPTIONAL);
 
-        private CType SubstType(CType typeSrc, TypeArray typeArgsCls, TypeArray typeArgsMeth, SubstTypeFlags grfst)
+        private CType SubstType(CType typeSrc, TypeArray typeArgsCls, TypeArray typeArgsMeth, bool denormMeth)
         {
-            if (typeSrc == null)
-                return null;
-
-            var ctx = new SubstContext(typeArgsCls, typeArgsMeth, grfst);
-            return ctx.FNop() ? typeSrc : SubstTypeCore(typeSrc, ctx);
+            Debug.Assert(typeSrc != null);
+            SubstContext ctx = new SubstContext(typeArgsCls, typeArgsMeth, denormMeth);
+            return ctx.IsNop ? typeSrc : SubstTypeCore(typeSrc, ctx);
         }
 
         public AggregateType SubstType(AggregateType typeSrc, TypeArray typeArgsCls)
         {
-            if (typeSrc != null)
-            {
-                SubstContext ctx = new SubstContext(typeArgsCls, null, SubstTypeFlags.NormNone);
-                if (!ctx.FNop())
-                {
-                    return SubstTypeCore(typeSrc, ctx);
-                }
-            }
+            Debug.Assert(typeSrc != null);
 
-            return typeSrc;
+            SubstContext ctx = new SubstContext(typeArgsCls, null, false);
+            return ctx.IsNop ? typeSrc : SubstTypeCore(typeSrc, ctx);
         }
 
-        private CType SubstType(CType typeSrc, TypeArray typeArgsCls, TypeArray typeArgsMeth)
-        {
-            return SubstType(typeSrc, typeArgsCls, typeArgsMeth, SubstTypeFlags.NormNone);
-        }
+        private CType SubstType(CType typeSrc, TypeArray typeArgsCls, TypeArray typeArgsMeth) =>
+            SubstType(typeSrc, typeArgsCls, typeArgsMeth, false);
 
         public TypeArray SubstTypeArray(TypeArray taSrc, SubstContext ctx)
         {
-            if (taSrc != null && taSrc.Count != 0 && ctx != null && !ctx.FNop())
+            if (taSrc != null && taSrc.Count != 0 && ctx != null && !ctx.IsNop)
             {
                 CType[] srcs = taSrc.Items;
                 for (int i = 0; i < srcs.Length; i++)
@@ -260,7 +248,7 @@ namespace Microsoft.CSharp.RuntimeBinder.Semantics
         public TypeArray SubstTypeArray(TypeArray taSrc, TypeArray typeArgsCls, TypeArray typeArgsMeth)
             => taSrc == null || taSrc.Count == 0
             ? taSrc
-            : SubstTypeArray(taSrc, new SubstContext(typeArgsCls, typeArgsMeth, SubstTypeFlags.NormNone));
+            : SubstTypeArray(taSrc, new SubstContext(typeArgsCls, typeArgsMeth, false));
 
         public TypeArray SubstTypeArray(TypeArray taSrc, TypeArray typeArgsCls) => SubstTypeArray(taSrc, typeArgsCls, null);
 
@@ -323,41 +311,40 @@ namespace Microsoft.CSharp.RuntimeBinder.Semantics
                         int index = tvs.GetIndexInTotalParameters();
                         if (tvs.IsMethodTypeParameter())
                         {
-                            if ((pctx.grfst & SubstTypeFlags.DenormMeth) != 0 && tvs.parent != null)
-                                return type;
-                            Debug.Assert(tvs.GetIndexInOwnParameters() == tvs.GetIndexInTotalParameters());
-                            if (index < pctx.ctypeMeth)
+                            if (pctx.DenormMeth && tvs.parent != null)
                             {
-                                Debug.Assert(pctx.prgtypeMeth != null);
-                                return pctx.prgtypeMeth[index];
+                                return type;
                             }
-                            else
+
+                            Debug.Assert(tvs.GetIndexInOwnParameters() == tvs.GetIndexInTotalParameters());
+                            if (index < pctx.MethodTypes.Length)
                             {
-                                return ((pctx.grfst & SubstTypeFlags.NormMeth) != 0 ? GetStdMethTypeVar(index) : type);
+                                Debug.Assert(pctx.MethodTypes != null);
+                                return pctx.MethodTypes[index];
                             }
-                        }
-                        if ((pctx.grfst & SubstTypeFlags.DenormClass) != 0 && tvs.parent != null)
+
                             return type;
-                        return index < pctx.ctypeCls ? pctx.prgtypeCls[index] :
-                               ((pctx.grfst & SubstTypeFlags.NormClass) != 0 ? GetStdClsTypeVar(index) : type);
+                        }
+
+                        return index < pctx.ClassTypes.Length ? pctx.ClassTypes[index] : type;
                     }
             }
         }
 
-        public bool SubstEqualTypes(CType typeDst, CType typeSrc, TypeArray typeArgsCls, TypeArray typeArgsMeth, SubstTypeFlags grfst)
+        public bool SubstEqualTypes(CType typeDst, CType typeSrc, TypeArray typeArgsCls, TypeArray typeArgsMeth, bool denormMeth)
         {
             if (typeDst.Equals(typeSrc))
             {
-                Debug.Assert(typeDst.Equals(SubstType(typeSrc, typeArgsCls, typeArgsMeth, grfst)));
+                Debug.Assert(typeDst.Equals(SubstType(typeSrc, typeArgsCls, typeArgsMeth, denormMeth)));
                 return true;
             }
 
-            var ctx = new SubstContext(typeArgsCls, typeArgsMeth, grfst);
+            SubstContext ctx = new SubstContext(typeArgsCls, typeArgsMeth, denormMeth);
 
-            return !ctx.FNop() && SubstEqualTypesCore(typeDst, typeSrc, ctx);
+            return !ctx.IsNop && SubstEqualTypesCore(typeDst, typeSrc, ctx);
         }
 
-        public bool SubstEqualTypeArrays(TypeArray taDst, TypeArray taSrc, TypeArray typeArgsCls, TypeArray typeArgsMeth, SubstTypeFlags grfst)
+        public bool SubstEqualTypeArrays(TypeArray taDst, TypeArray taSrc, TypeArray typeArgsCls, TypeArray typeArgsMeth)
         {
             // Handle the simple common cases first.
             if (taDst == taSrc || (taDst != null && taDst.Equals(taSrc)))
@@ -375,10 +362,12 @@ namespace Microsoft.CSharp.RuntimeBinder.Semantics
             if (taDst.Count == 0)
                 return true;
 
-            var ctx = new SubstContext(typeArgsCls, typeArgsMeth, grfst);
+            var ctx = new SubstContext(typeArgsCls, typeArgsMeth, true);
 
-            if (ctx.FNop())
+            if (ctx.IsNop)
+            {
                 return false;
+            }
 
             for (int i = 0; i < taDst.Count; i++)
             {
@@ -417,10 +406,11 @@ namespace Microsoft.CSharp.RuntimeBinder.Semantics
                     goto LCheckBases;
 
                 case TypeKind.TK_ParameterModifierType:
-                    if (!(typeDst is ParameterModifierType modDest) ||
-                        ((pctx.grfst & SubstTypeFlags.NoRefOutDifference) == 0 &&
-                         modDest.IsOut != ((ParameterModifierType)typeSrc).IsOut))
+                    if (!(typeDst is ParameterModifierType modDest) || modDest.IsOut != ((ParameterModifierType)typeSrc).IsOut)
+                    {
                         return false;
+                    }
+
                     goto LCheckBases;
 
                 case TypeKind.TK_PointerType:
@@ -459,36 +449,27 @@ namespace Microsoft.CSharp.RuntimeBinder.Semantics
 
                         if (tvs.IsMethodTypeParameter())
                         {
-                            if ((pctx.grfst & SubstTypeFlags.DenormMeth) != 0 && tvs.parent != null)
+                            if (pctx.DenormMeth && tvs.parent != null)
                             {
                                 // typeDst == typeSrc was handled above.
                                 Debug.Assert(typeDst != typeSrc);
                                 return false;
                             }
-                            Debug.Assert(tvs.GetIndexInOwnParameters() == tvs.GetIndexInTotalParameters());
-                            Debug.Assert(pctx.prgtypeMeth == null || tvs.GetIndexInTotalParameters() < pctx.ctypeMeth);
-                            if (index < pctx.ctypeMeth && pctx.prgtypeMeth != null)
-                            {
-                                return typeDst == pctx.prgtypeMeth[index];
-                            }
-                            if ((pctx.grfst & SubstTypeFlags.NormMeth) != 0)
+
+                            Debug.Assert(tvs.GetIndexInOwnParameters() == index);
+                            Debug.Assert(tvs.GetIndexInTotalParameters() < pctx.MethodTypes.Length);
+                            if (index < pctx.MethodTypes.Length)
                             {
-                                return typeDst == GetStdMethTypeVar(index);
+                                return typeDst == pctx.MethodTypes[index];
                             }
                         }
                         else
                         {
-                            if ((pctx.grfst & SubstTypeFlags.DenormClass) != 0 && tvs.parent != null)
+                            Debug.Assert(index < pctx.ClassTypes.Length);
+                            if (index < pctx.ClassTypes.Length)
                             {
-                                // typeDst == typeSrc was handled above.
-                                Debug.Assert(typeDst != typeSrc);
-                                return false;
+                                return typeDst == pctx.ClassTypes[index];
                             }
-                            Debug.Assert(pctx.prgtypeCls == null || tvs.GetIndexInTotalParameters() < pctx.ctypeCls);
-                            if (index < pctx.ctypeCls)
-                                return typeDst == pctx.prgtypeCls[index];
-                            if ((pctx.grfst & SubstTypeFlags.NormClass) != 0)
-                                return typeDst == GetStdClsTypeVar(index);
                         }
                     }
                     return false;
@@ -618,12 +599,10 @@ namespace Microsoft.CSharp.RuntimeBinder.Semantics
         }
 
         public AggregateType SubstType(AggregateType typeSrc, SubstContext ctx) =>
-            ctx == null || ctx.FNop() ? typeSrc : SubstTypeCore(typeSrc, ctx);
+            ctx == null || ctx.IsNop ? typeSrc : SubstTypeCore(typeSrc, ctx);
 
-        public CType SubstType(CType typeSrc, SubstContext pctx)
-        {
-            return (pctx == null || pctx.FNop()) ? typeSrc : SubstTypeCore(typeSrc, pctx);
-        }
+        public CType SubstType(CType typeSrc, SubstContext pctx) =>
+            pctx == null || pctx.IsNop ? typeSrc : SubstTypeCore(typeSrc, pctx);
 
         public CType SubstType(CType typeSrc, AggregateType atsCls)
         {
@@ -650,31 +629,19 @@ namespace Microsoft.CSharp.RuntimeBinder.Semantics
             return this.SubstTypeArray(taSrc, atsCls, (TypeArray)null);
         }
 
-        private bool SubstEqualTypes(CType typeDst, CType typeSrc, CType typeCls, TypeArray typeArgsMeth)
-        {
-            return SubstEqualTypes(typeDst, typeSrc, (typeCls as AggregateType)?.TypeArgsAll, typeArgsMeth, SubstTypeFlags.NormNone);
-        }
+        private bool SubstEqualTypes(CType typeDst, CType typeSrc, CType typeCls, TypeArray typeArgsMeth) =>
+            SubstEqualTypes(typeDst, typeSrc, (typeCls as AggregateType)?.TypeArgsAll, typeArgsMeth, false);
 
         public bool SubstEqualTypes(CType typeDst, CType typeSrc, CType typeCls)
         {
             return SubstEqualTypes(typeDst, typeSrc, typeCls, (TypeArray)null);
         }
 
-        //public bool SubstEqualTypeArrays(TypeArray taDst, TypeArray taSrc, AggregateType atsCls, TypeArray typeArgsMeth)
-        //{
-        //    return SubstEqualTypeArrays(taDst, taSrc, atsCls != null ? atsCls.TypeArgsAll : (TypeArray)null, typeArgsMeth, SubstTypeFlags.NormNone);
-        //}
-
         public TypeParameterType GetStdMethTypeVar(int iv)
         {
             return _stvcMethod.GetTypeVarSym(iv, this, true);
         }
 
-        private TypeParameterType GetStdClsTypeVar(int iv)
-        {
-            return _stvcClass.GetTypeVarSym(iv, this, false);
-        }
-
         // These are singletons for each.
         public TypeParameterType GetTypeParameter(TypeParameterSymbol pSymbol)
         {