From aaa30591a86442e27392cfe569d397bee3aad9f2 Mon Sep 17 00:00:00 2001 From: dougbu Date: Thu, 27 Mar 2014 22:21:26 -0700 Subject: [PATCH] Get things working in our world - usual stuff, especially use of `var` and `[NotNull]` - remove references to `ExpressionFingerprintChain` and so on to minimize classes we bring over now (and remove one cache) - copy over missing resource - rework checks in `IsSingleArgumentIndexer()` --- .../Expressions/CachedExpressionCompiler.cs | 62 +++++----------- .../Expressions/ExpressionHelper.cs | 70 ++++++++++--------- .../Properties/Resources.Designer.cs | 16 +++++ .../Properties/Resources.resx | 3 + .../project.json | 1 + 5 files changed, 74 insertions(+), 78 deletions(-) diff --git a/src/Microsoft.AspNet.Mvc.Rendering/Expressions/CachedExpressionCompiler.cs b/src/Microsoft.AspNet.Mvc.Rendering/Expressions/CachedExpressionCompiler.cs index a0625601e9..82cfab6c40 100644 --- a/src/Microsoft.AspNet.Mvc.Rendering/Expressions/CachedExpressionCompiler.cs +++ b/src/Microsoft.AspNet.Mvc.Rendering/Expressions/CachedExpressionCompiler.cs @@ -1,20 +1,19 @@ -// Copyright (c) Microsoft Open Technologies, Inc. All rights reserved. See License.txt in the project root for license information. - +using System; using System.Collections.Concurrent; -using System.Collections.Generic; using System.Linq.Expressions; using System.Reflection; -namespace System.Web.Mvc.ExpressionUtil +namespace Microsoft.AspNet.Mvc.Rendering.Expressions { - internal static class CachedExpressionCompiler + public static class CachedExpressionCompiler { // This is the entry point to the cached expression compilation system. The system // will try to turn the expression into an actual delegate as quickly as possible, // relying on cache lookups and other techniques to save time if appropriate. // If the provided expression is particularly obscure and the system doesn't know // how to handle it, we'll just compile the expression as normal. - public static Func Process(Expression> lambdaExpression) + public static Func Process( + [NotNull] Expression> lambdaExpression) { return Compiler.Compile(lambdaExpression); } @@ -29,39 +28,35 @@ namespace System.Web.Mvc.ExpressionUtil private static readonly ConcurrentDictionary> _constMemberAccessDict = new ConcurrentDictionary>(); - private static readonly ConcurrentDictionary> _fingerprintedCache = - new ConcurrentDictionary>(); - - public static Func Compile(Expression> expr) + public static Func Compile([NotNull] Expression> expr) { return CompileFromIdentityFunc(expr) ?? CompileFromConstLookup(expr) ?? CompileFromMemberAccess(expr) - ?? CompileFromFingerprint(expr) ?? CompileSlow(expr); } - private static Func CompileFromConstLookup(Expression> expr) + private static Func CompileFromConstLookup([NotNull] Expression> expr) { - ConstantExpression constExpr = expr.Body as ConstantExpression; + var constExpr = expr.Body as ConstantExpression; if (constExpr != null) { // model => {const} - TOut constantValue = (TOut)constExpr.Value; + var constantValue = (TOut)constExpr.Value; return _ => constantValue; } return null; } - private static Func CompileFromIdentityFunc(Expression> expr) + private static Func CompileFromIdentityFunc([NotNull] Expression> expr) { if (expr.Body == expr.Parameters[0]) { // model => model - // don't need to lock, as all identity funcs are identical + // Don't need to lock, as all identity funcs are identical. if (_identityFunc == null) { _identityFunc = expr.Compile(); @@ -73,29 +68,7 @@ namespace System.Web.Mvc.ExpressionUtil return null; } - private static Func CompileFromFingerprint(Expression> expr) - { - List capturedConstants; - ExpressionFingerprintChain fingerprint = FingerprintingExpressionVisitor.GetFingerprintChain(expr, out capturedConstants); - - if (fingerprint != null) - { - var del = _fingerprintedCache.GetOrAdd(fingerprint, _ => - { - // Fingerprinting succeeded, but there was a cache miss. Rewrite the expression - // and add the rewritten expression to the cache. - - var hoistedExpr = HoistingExpressionVisitor.Hoist(expr); - return hoistedExpr.Compile(); - }); - return model => del(model, capturedConstants); - } - - // couldn't be fingerprinted - return null; - } - - private static Func CompileFromMemberAccess(Expression> expr) + private static Func CompileFromMemberAccess([NotNull] Expression> expr) { // Performance tests show that on the x64 platform, special-casing static member and // captured local variable accesses is faster than letting the fingerprinting system @@ -103,7 +76,7 @@ namespace System.Web.Mvc.ExpressionUtil // by around one microsecond, so it's not worth it to complicate the logic here with // an architecture check. - MemberExpression memberExpr = expr.Body as MemberExpression; + var memberExpr = expr.Body as MemberExpression; if (memberExpr != null) { if (memberExpr.Expression == expr.Parameters[0] || memberExpr.Expression == null) @@ -112,7 +85,7 @@ namespace System.Web.Mvc.ExpressionUtil return _simpleMemberAccessDict.GetOrAdd(memberExpr.Member, _ => expr.Compile()); } - ConstantExpression constExpr = memberExpr.Expression as ConstantExpression; + var constExpr = memberExpr.Expression as ConstantExpression; if (constExpr != null) { // model => {const}.Member (captured local variable) @@ -122,11 +95,12 @@ namespace System.Web.Mvc.ExpressionUtil var constParamExpr = Expression.Parameter(typeof(object), "capturedLocal"); var constCastExpr = Expression.Convert(constParamExpr, memberExpr.Member.DeclaringType); var newMemberAccessExpr = memberExpr.Update(constCastExpr); - var newLambdaExpr = Expression.Lambda>(newMemberAccessExpr, constParamExpr); + var newLambdaExpr = + Expression.Lambda>(newMemberAccessExpr, constParamExpr); return newLambdaExpr.Compile(); }); - object capturedLocal = constExpr.Value; + var capturedLocal = constExpr.Value; return _ => del(capturedLocal); } } @@ -134,7 +108,7 @@ namespace System.Web.Mvc.ExpressionUtil return null; } - private static Func CompileSlow(Expression> expr) + private static Func CompileSlow([NotNull] Expression> expr) { // fallback compilation system - just compile the expression directly return expr.Compile(); diff --git a/src/Microsoft.AspNet.Mvc.Rendering/Expressions/ExpressionHelper.cs b/src/Microsoft.AspNet.Mvc.Rendering/Expressions/ExpressionHelper.cs index be91bcd8bc..87ea03af27 100644 --- a/src/Microsoft.AspNet.Mvc.Rendering/Expressions/ExpressionHelper.cs +++ b/src/Microsoft.AspNet.Mvc.Rendering/Expressions/ExpressionHelper.cs @@ -1,36 +1,34 @@ -// Copyright (c) Microsoft Open Technologies, Inc. All rights reserved. See License.txt in the project root for license information. - +using System; using System.Collections.Generic; using System.Globalization; using System.Linq; using System.Linq.Expressions; using System.Reflection; -using System.Web.Mvc.ExpressionUtil; -using System.Web.Mvc.Properties; -namespace System.Web.Mvc +namespace Microsoft.AspNet.Mvc.Rendering.Expressions { public static class ExpressionHelper { public static string GetExpressionText(string expression) { + // If it's exactly "model", then give them an empty string, to replicate the lambda behavior. return - String.Equals(expression, "model", StringComparison.OrdinalIgnoreCase) - ? String.Empty // If it's exactly "model", then give them an empty string, to replicate the lambda behavior + string.Equals(expression, "model", StringComparison.OrdinalIgnoreCase) + ? string.Empty : expression; } - public static string GetExpressionText(LambdaExpression expression) + public static string GetExpressionText([NotNull] LambdaExpression expression) { // Split apart the expression string for property/field accessors to create its name - Stack nameParts = new Stack(); - Expression part = expression.Body; + var nameParts = new Stack(); + var part = expression.Body; while (part != null) { if (part.NodeType == ExpressionType.Call) { - MethodCallExpression methodExpression = (MethodCallExpression)part; + var methodExpression = (MethodCallExpression)part; if (!IsSingleArgumentIndexer(methodExpression)) { @@ -46,7 +44,7 @@ namespace System.Web.Mvc } else if (part.NodeType == ExpressionType.ArrayIndex) { - BinaryExpression binaryExpression = (BinaryExpression)part; + var binaryExpression = (BinaryExpression)part; nameParts.Push( GetIndexerInvocation( @@ -57,17 +55,16 @@ namespace System.Web.Mvc } else if (part.NodeType == ExpressionType.MemberAccess) { - MemberExpression memberExpressionPart = (MemberExpression)part; + var memberExpressionPart = (MemberExpression)part; nameParts.Push("." + memberExpressionPart.Member.Name); part = memberExpressionPart.Expression; } else if (part.NodeType == ExpressionType.Parameter) { - // Dev10 Bug #907611 // When the expression is parameter based (m => m.Something...), we'll push an empty // string onto the stack and stop evaluating. The extra empty string makes sure that // we don't accidentally cut off too much of m => m.Model. - nameParts.Push(String.Empty); + nameParts.Push(string.Empty); part = null; } else @@ -77,7 +74,7 @@ namespace System.Web.Mvc } // If it starts with "model", then strip that away - if (nameParts.Count > 0 && String.Equals(nameParts.Peek(), ".model", StringComparison.OrdinalIgnoreCase)) + if (nameParts.Count > 0 && string.Equals(nameParts.Peek(), ".model", StringComparison.OrdinalIgnoreCase)) { nameParts.Pop(); } @@ -87,14 +84,15 @@ namespace System.Web.Mvc return nameParts.Aggregate((left, right) => left + right).TrimStart('.'); } - return String.Empty; + return string.Empty; } - private static string GetIndexerInvocation(Expression expression, ParameterExpression[] parameters) + private static string GetIndexerInvocation([NotNull] Expression expression, + [NotNull] ParameterExpression[] parameters) { - Expression converted = Expression.Convert(expression, typeof(object)); - ParameterExpression fakeParameter = Expression.Parameter(typeof(object), null); - Expression> lambda = Expression.Lambda>(converted, fakeParameter); + var converted = Expression.Convert(expression, typeof(object)); + var fakeParameter = Expression.Parameter(typeof(object), null); + var lambda = Expression.Lambda>(converted, fakeParameter); Func func; try @@ -104,30 +102,34 @@ namespace System.Web.Mvc catch (InvalidOperationException ex) { throw new InvalidOperationException( - String.Format( - CultureInfo.CurrentCulture, - MvcResources.ExpressionHelper_InvalidIndexerExpression, - expression, - parameters[0].Name), - ex); + Resources.FormatExpressionHelper_InvalidIndexerExpression(expression, parameters[0].Name), + ex); } return "[" + Convert.ToString(func(null), CultureInfo.InvariantCulture) + "]"; } - internal static bool IsSingleArgumentIndexer(Expression expression) + public static bool IsSingleArgumentIndexer(Expression expression) { - MethodCallExpression methodExpression = expression as MethodCallExpression; + var methodExpression = expression as MethodCallExpression; if (methodExpression == null || methodExpression.Arguments.Count != 1) { return false; } - return methodExpression.Method - .DeclaringType - .GetDefaultMembers() - .OfType() - .Any(p => p.GetGetMethod() == methodExpression.Method); + // Check whether GetDefaultMembers() (if present in CoreCLR) would return a member of this type. Compiler + // names the indexer property, if any, in a generated [DefaultMember] attribute for the containing type. + var declaringType = methodExpression.Method.DeclaringType; + var defaultMember = declaringType.GetTypeInfo().GetCustomAttribute(inherit: true); + if (defaultMember == null) + { + return false; + } + + // Find default property (the indexer) and confirm its getter is the method in this expression. + return declaringType.GetRuntimeProperties().Any( + property => (string.Equals(defaultMember.MemberName, property.Name, StringComparison.Ordinal) && + property.GetMethod == methodExpression.Method)); } } } diff --git a/src/Microsoft.AspNet.Mvc.Rendering/Properties/Resources.Designer.cs b/src/Microsoft.AspNet.Mvc.Rendering/Properties/Resources.Designer.cs index a080c51674..b64358607e 100644 --- a/src/Microsoft.AspNet.Mvc.Rendering/Properties/Resources.Designer.cs +++ b/src/Microsoft.AspNet.Mvc.Rendering/Properties/Resources.Designer.cs @@ -90,6 +90,22 @@ namespace Microsoft.AspNet.Mvc.Rendering return GetString("DynamicViewData_ViewDataNull"); } + /// + /// The expression compiler was unable to evaluate the indexer expression '{0}' because it references the model parameter '{1}' which is unavailable. + /// + internal static string ExpressionHelper_InvalidIndexerExpression + { + get { return GetString("ExpressionHelper_InvalidIndexerExpression"); } + } + + /// + /// The expression compiler was unable to evaluate the indexer expression '{0}' because it references the model parameter '{1}' which is unavailable. + /// + internal static string FormatExpressionHelper_InvalidIndexerExpression(object p0, object p1) + { + return string.Format(CultureInfo.CurrentCulture, GetString("ExpressionHelper_InvalidIndexerExpression"), p0, p1); + } + /// /// Must call 'Contextualize' method before using this HtmlHelper instance. /// diff --git a/src/Microsoft.AspNet.Mvc.Rendering/Properties/Resources.resx b/src/Microsoft.AspNet.Mvc.Rendering/Properties/Resources.resx index deb8e5ac34..107f5f213b 100644 --- a/src/Microsoft.AspNet.Mvc.Rendering/Properties/Resources.resx +++ b/src/Microsoft.AspNet.Mvc.Rendering/Properties/Resources.resx @@ -132,6 +132,9 @@ ViewData value must not be null. + + The expression compiler was unable to evaluate the indexer expression '{0}' because it references the model parameter '{1}' which is unavailable. + Must call 'Contextualize' method before using this HtmlHelper instance. diff --git a/src/Microsoft.AspNet.Mvc.Rendering/project.json b/src/Microsoft.AspNet.Mvc.Rendering/project.json index 0e9d1cda7c..d70f9b4f31 100644 --- a/src/Microsoft.AspNet.Mvc.Rendering/project.json +++ b/src/Microsoft.AspNet.Mvc.Rendering/project.json @@ -12,6 +12,7 @@ "dependencies": { "Microsoft.CSharp": "4.0.0.0", "System.Collections": "4.0.0.0", + "System.Collections.Concurrent": "4.0.0.0", "System.ComponentModel": "4.0.0.0", "System.Diagnostics.Contracts": "4.0.0.0", "System.Diagnostics.Debug": "4.0.10.0",