Eval cond perf

src/Build/Evaluation/Conditionals/GenericExpressionNode.cs

@@ -18,6 +18,9 @@ internal abstract class GenericExpressionNode
         internal abstract bool BoolEvaluate(ConditionEvaluator.IConditionEvaluationState state);
         internal abstract double NumericEvaluate(ConditionEvaluator.IConditionEvaluationState state);
         internal abstract Version VersionEvaluate(ConditionEvaluator.IConditionEvaluationState state);
+        internal abstract bool TryBoolEvaluate(ConditionEvaluator.IConditionEvaluationState state, out bool result);
+        internal abstract bool TryNumericEvaluate(ConditionEvaluator.IConditionEvaluationState state, out double result);
+        internal abstract bool TryVersionEvaluate(ConditionEvaluator.IConditionEvaluationState state, out Version result);
 
         /// <summary>
         /// Returns true if this node evaluates to an empty string,

src/Build/Evaluation/Conditionals/MultipleComparisonExpressionNode.cs

@@ -48,36 +48,41 @@ internal override bool BoolEvaluate(ConditionEvaluator.IConditionEvaluationState
             // and we know which do, then we already have enough information to evaluate this expression.
             // That means we don't have to fully expand a condition like " '@(X)' == '' " 
             // which is a performance advantage if @(X) is a huge item list.
-            if (LeftChild.EvaluatesToEmpty(state) || RightChild.EvaluatesToEmpty(state))
+            bool leftEmpty = LeftChild.EvaluatesToEmpty(state);
+            bool rightEmpty = RightChild.EvaluatesToEmpty(state);
+            if (leftEmpty || rightEmpty)
             {
                 UpdateConditionedProperties(state);
 
-                return Compare(LeftChild.EvaluatesToEmpty(state), RightChild.EvaluatesToEmpty(state));
+                return Compare(leftEmpty, rightEmpty);
             }
-
-            if (LeftChild.CanNumericEvaluate(state) && RightChild.CanNumericEvaluate(state))
+            else if (LeftChild.TryNumericEvaluate(state, out double leftNumericValue) && RightChild.TryNumericEvaluate(state, out double rightNumericValue))
             {
-                return Compare(LeftChild.NumericEvaluate(state), RightChild.NumericEvaluate(state));
+                // The left child evaluating to a number and the right child not evaluating to a number
+                // is insufficient to say they are not equal because $(MSBuildToolsVersion) evaluates to
+                // the string "Current" most of the time but when doing numeric comparisons, is treated
+                // as a version and returns "17.0" (or whatever the current tools version is). This means
+                // that if '$(MSBuildToolsVersion)' is "equal" to BOTH '17.0' and 'Current' (if 'Current'
+                // is 17.0).
+                return Compare(leftNumericValue, rightNumericValue);
             }
-            else if (LeftChild.CanBoolEvaluate(state) && RightChild.CanBoolEvaluate(state))
+            else if (LeftChild.TryBoolEvaluate(state, out bool leftBoolValue) && RightChild.TryBoolEvaluate(state, out bool rightBoolValue))
             {
-                return Compare(LeftChild.BoolEvaluate(state), RightChild.BoolEvaluate(state));
+                return Compare(leftBoolValue, rightBoolValue);
             }
-            else // string comparison
-            {
-                string leftExpandedValue = LeftChild.GetExpandedValue(state);
-                string rightExpandedValue = RightChild.GetExpandedValue(state);
 
-                ProjectErrorUtilities.VerifyThrowInvalidProject
-                    (leftExpandedValue != null && rightExpandedValue != null,
-                     state.ElementLocation,
-                     "IllFormedCondition",
-                     state.Condition);
+            string leftExpandedValue = LeftChild.GetExpandedValue(state);
+            string rightExpandedValue = RightChild.GetExpandedValue(state);
 
-                UpdateConditionedProperties(state);
+            ProjectErrorUtilities.VerifyThrowInvalidProject
+                (leftExpandedValue != null && rightExpandedValue != null,
+                    state.ElementLocation,
+                    "IllFormedCondition",
+                    state.Condition);
 
-                return Compare(leftExpandedValue, rightExpandedValue);
-            }
+            UpdateConditionedProperties(state);
+
+            return Compare(leftExpandedValue, rightExpandedValue);
         }
 
         /// <summary>

src/Build/Evaluation/Conditionals/NumericExpressionNode.cs

@@ -73,8 +73,23 @@ internal override bool CanVersionEvaluate(ConditionEvaluator.IConditionEvaluatio
         {
             // Check if the value can be formatted as a Version number
             // This is needed for nodes that identify as Numeric but can't be parsed as numbers (e.g. 8.1.1.0 vs 8.1)
-            Version unused;
-            return Version.TryParse(_value, out unused);
+            return Version.TryParse(_value, out _);
+        }
+
+        internal override bool TryBoolEvaluate(ConditionEvaluator.IConditionEvaluationState state, out bool result)
+        {
+            result = default;
+            return false;
+        }
+
+        internal override bool TryNumericEvaluate(ConditionEvaluator.IConditionEvaluationState state, out double result)
+        {
+            return ConversionUtilities.TryConvertDecimalOrHexToDouble(_value, out result);
+        }
+
+        internal override bool TryVersionEvaluate(ConditionEvaluator.IConditionEvaluationState state, out Version result)
+        {
+            return Version.TryParse(_value, out result);
         }
 
         /// <summary>

src/Build/Evaluation/Conditionals/OperatorExpressionNode.cs

@@ -56,6 +56,24 @@ internal override bool CanVersionEvaluate(ConditionEvaluator.IConditionEvaluatio
             return false;
         }
 
+        internal override bool TryBoolEvaluate(ConditionEvaluator.IConditionEvaluationState state, out bool result)
+        {
+            result = BoolEvaluate(state);
+            return true;
+        }
+
+        internal override bool TryNumericEvaluate(ConditionEvaluator.IConditionEvaluationState state, out double result)
+        {
+            result = default;
+            return false;
+        }
+
+        internal override bool TryVersionEvaluate(ConditionEvaluator.IConditionEvaluationState state, out Version result)
+        {
+            result = default;
+            return false;
+        }
+
         /// <summary>
         /// Value after any item and property expressions are expanded
         /// </summary>

src/Build/Evaluation/Conditionals/StringExpressionNode.cs

@@ -3,7 +3,6 @@
 
 using System;
 using System.Diagnostics;
-
 using Microsoft.Build.Shared;
 
 namespace Microsoft.Build.Evaluation
@@ -85,6 +84,37 @@ internal override bool CanVersionEvaluate(ConditionEvaluator.IConditionEvaluatio
             return Version.TryParse(GetExpandedValue(state), out _);
         }
 
+        internal override bool TryBoolEvaluate(ConditionEvaluator.IConditionEvaluationState state, out bool result)
+        {
+            return ConversionUtilities.TryConvertStringToBool(GetExpandedValue(state), out result);
+        }
+
+        internal override bool TryNumericEvaluate(ConditionEvaluator.IConditionEvaluationState state, out double result)
+        {
+            if (ShouldBeTreatedAsVisualStudioVersion(state))
+            {
+                result = ConversionUtilities.ConvertDecimalOrHexToDouble(MSBuildConstants.CurrentVisualStudioVersion);
+                return true;
+            }
+            else
+            {
+                return ConversionUtilities.TryConvertDecimalOrHexToDouble(GetExpandedValue(state), out result);
+            }
+        }
+
+        internal override bool TryVersionEvaluate(ConditionEvaluator.IConditionEvaluationState state, out Version result)
+        {
+            if (ShouldBeTreatedAsVisualStudioVersion(state))
+            {
+                result = Version.Parse(MSBuildConstants.CurrentVisualStudioVersion);
+                return true;
+            }
+            else
+            {
+                return Version.TryParse(GetExpandedValue(state), out result);
+            }
+        }
+
         /// <summary>
         /// Returns true if this node evaluates to an empty string,
         /// otherwise false.
@@ -98,6 +128,25 @@ internal override bool EvaluatesToEmpty(ConditionEvaluator.IConditionEvaluationS
             {
                 if (_expandable)
                 {
+                    switch (_value.Length)
+                    {
+                        case 0:
+                            _cachedExpandedValue = String.Empty;
+                            return true;
+                        // If the length is 1 or 2, it can't possibly be a property, item, or metadata, and it isn't empty.
+                        case 1:
+                        case 2:
+                            _cachedExpandedValue = _value;
+                            return false;
+                        default:
+                            if (_value[1] != '(' || _value[_value.Length - 1] != ')' || (_value[0] != '$' && _value[0] != '%' && _value[0] != '@'))
+                            {
+                                // This isn't just a property, item, or metadata value, and it isn't empty.
+                                return false;
+                            }
+                            break;
+                    }
+
                     string expandBreakEarly = state.ExpandIntoStringBreakEarly(_value);
 
                     if (expandBreakEarly == null)

src/Build/Evaluation/ExpressionShredder.cs

@@ -110,12 +110,14 @@ internal static List<ItemExpressionCapture> GetReferencedItemExpressions(string
         {
             List<ItemExpressionCapture> subExpressions = null;
 
-            if (expression.IndexOf('@') < 0)
+            int startInd = expression.IndexOf('@', start, end - start);
+
+            if (startInd < 0)
             {
                 return null;
             }
 
-            for (int i = start; i < end; i++)
+            for (int i = startInd; i < end; i++)
             {
                 int restartPoint;
                 int startPoint;

src/Shared/ConversionUtilities.cs

@@ -64,6 +64,21 @@ internal static string ConvertByteArrayToHex(byte[] bytes)
             return sb.ToString();
         }
 
+        internal static bool TryConvertStringToBool(string parameterValue, out bool boolValue)
+        {
+            boolValue = false;
+            if (ValidBooleanTrue(parameterValue))
+            {
+                boolValue = true;
+                return true;
+            }
+            else if (ValidBooleanFalse(parameterValue))
+            {
+                return true;
+            }
+            return false;
+        }
+
         /// <summary>
         /// Returns true if the string can be successfully converted to a bool,
         /// such as "on" or "yes"
@@ -123,30 +138,40 @@ internal static double ConvertHexToDouble(string number)
         /// </summary>
         internal static double ConvertDecimalOrHexToDouble(string number)
         {
-            if (ConversionUtilities.ValidDecimalNumber(number))
+            if (TryConvertDecimalOrHexToDouble(number, out double result))
             {
-                return ConversionUtilities.ConvertDecimalToDouble(number);
+                return result;
             }
-            else if (ConversionUtilities.ValidHexNumber(number))
+            ErrorUtilities.VerifyThrow(false, "Cannot numeric evaluate");
+            return 0.0D;
+        }
+
+        internal static bool TryConvertDecimalOrHexToDouble(string number, out double doubleValue)
+        {
+            if (ConversionUtilities.ValidDecimalNumber(number, out doubleValue))
             {
-                return ConversionUtilities.ConvertHexToDouble(number);
+                return true;
+            }
+            else if (ConversionUtilities.ValidHexNumber(number, out int hexValue))
+            {
+                doubleValue = (double)hexValue;
+                return true;
             }
             else
             {
-                ErrorUtilities.VerifyThrow(false, "Cannot numeric evaluate");
-                return 0.0D;
+                return false;
             }
         }
 
         /// <summary>
         /// Returns true if the string is a valid hex number, like "0xABC"
         /// </summary>
-        private static bool ValidHexNumber(string number)
+        private static bool ValidHexNumber(string number, out int value)
         {
             bool canConvert = false;
+            value = 0;
             if (number.Length >= 3 && number[0] == '0' && (number[1] == 'x' || number[1] == 'X'))
             {
-                int value;
                 canConvert = Int32.TryParse(number.Substring(2), NumberStyles.AllowHexSpecifier, CultureInfo.InvariantCulture.NumberFormat, out value);
             }
             return canConvert;
@@ -155,9 +180,8 @@ private static bool ValidHexNumber(string number)
         /// <summary>
         /// Returns true if the string is a valid decimal number, like "-123.456"
         /// </summary>
-        private static bool ValidDecimalNumber(string number)
+        private static bool ValidDecimalNumber(string number, out double value)
         {
-            double value;
             return Double.TryParse(number, NumberStyles.AllowDecimalPoint | NumberStyles.AllowLeadingSign, CultureInfo.InvariantCulture.NumberFormat, out value) && !double.IsInfinity(value);
         }
 
@@ -166,7 +190,7 @@ private static bool ValidDecimalNumber(string number)
         /// </summary>
         internal static bool ValidDecimalOrHexNumber(string number)
         {
-            return ValidDecimalNumber(number) || ValidHexNumber(number);
+            return ValidDecimalNumber(number, out _) || ValidHexNumber(number, out _);
         }
     }
 }