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,47 @@ 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))
+            if (LeftChild.TryNumericEvaluate(state, out double leftNumericValue))
             {
-                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).
+                if (RightChild.TryNumericEvaluate(state, out double rightNumericValue))
+                {
+                    return Compare(leftNumericValue, rightNumericValue);
+                }
             }
-            else if (LeftChild.CanBoolEvaluate(state) && RightChild.CanBoolEvaluate(state))
+
+            if (LeftChild.TryBoolEvaluate(state, out bool leftBoolValue))
             {
-                return Compare(LeftChild.BoolEvaluate(state), RightChild.BoolEvaluate(state));
+                RightChild.TryBoolEvaluate(state, out bool rightBoolValue);
+                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

@@ -84,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.

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 _);
         }
     }
 }