Avoid overflow exceptions when converting BOOL to bool

This means it has to be a lossy conversion, since `BOOL` is 4 bytes and `bool` is 1 byte. Generally .NET recommends avoiding lossy implicity operators (though explicit is ok). But for a type like this, it seems like super-high value for folks to be able to author `if (!SomeBOOL())` and have that work without casts. Fixes #624
microsoft · Aug 9, 2022 · 55f4867 · 55f4867
1 parent 39757ab
commit 55f4867
Show file tree

Hide file tree

Showing 5 changed files with 88 additions and 62 deletions.
diff --git a/src/Microsoft.Windows.CsWin32/templates/BOOL.cs b/src/Microsoft.Windows.CsWin32/templates/BOOL.cs
@@ -1,11 +1,6 @@
 partial struct BOOL
 {
-	internal unsafe BOOL(bool value) => this.Value = *(sbyte*)&value;
-	public static unsafe implicit operator bool(BOOL value)
-	{
-		sbyte v = checked((sbyte)value.Value);
-		return *(bool*)&v;
-	}
-
+	internal BOOL(bool value) => this.Value = value ? 1 : 0;
+	public static implicit operator bool(BOOL value) => value.Value != 0;
 	public static implicit operator BOOL(bool value) => new BOOL(value);
 }
diff --git a/src/Microsoft.Windows.CsWin32/templates/BOOLEAN.cs b/src/Microsoft.Windows.CsWin32/templates/BOOLEAN.cs
@@ -1,11 +1,6 @@
 partial struct BOOLEAN
 {
-	internal unsafe BOOLEAN(bool value) => this.Value = *(byte*)&value;
-	public static unsafe implicit operator bool(BOOLEAN value)
-	{
-		byte v = checked((byte)value.Value);
-		return *(bool*)&v;
-	}
-
+	internal BOOLEAN(bool value) => this.Value = value ? (byte)1 : (byte)0;
+	public static implicit operator bool(BOOLEAN value) => value.Value != 0;
 	public static implicit operator BOOLEAN(bool value) => new BOOLEAN(value);
 }
diff --git a/test/GenerationSandbox.Tests/BoolTests.cs b/test/GenerationSandbox.Tests/BoolTests.cs
@@ -6,7 +6,7 @@
 public class BoolTests
 {
     [Fact]
-    public void Bool()
+    public void Ctor_bool()
     {
         BOOL b = true;
         bool b2 = b;
@@ -16,26 +16,31 @@ public void Bool()
         Assert.False(default(BOOL));
     }
 
-    [Theory]
-    [InlineData(3)]
-    [InlineData(-1)]
-    public void NotLossyConversionBetweenBoolAndBOOL(int ordinal)
+    [Fact]
+    public void Ctor_int()
     {
-        BOOL nativeBool = new BOOL(ordinal);
-        bool managedBool = nativeBool;
-        BOOL roundtrippedNativeBool = managedBool;
-        Assert.Equal(nativeBool, roundtrippedNativeBool);
+        Assert.Equal(2, new BOOL(2).Value);
+    }
+
+    [Fact]
+    public void ExplicitCast()
+    {
+        Assert.Equal(2, ((BOOL)2).Value);
     }
 
     [Theory]
     [InlineData(3)]
     [InlineData(-1)]
-    public void NotLossyConversionBetweenBoolAndBOOL_Ctors(int ordinal)
+    [InlineData(0)]
+    [InlineData(1)]
+    [InlineData(0xfffff)]
+    public void LossyConversionFromBOOLtoBool(int ordinal)
     {
         BOOL nativeBool = new BOOL(ordinal);
         bool managedBool = nativeBool;
-        BOOL roundtrippedNativeBool = new BOOL(managedBool);
-        Assert.Equal(nativeBool, roundtrippedNativeBool);
+        Assert.Equal(ordinal != 0, managedBool);
+        BOOLEAN roundtrippedNativeBool = managedBool;
+        Assert.Equal(managedBool ? 1 : 0, roundtrippedNativeBool);
     }
 
     [Fact]
@@ -56,5 +61,27 @@ public void BOOL_OverridesEqualityOperator()
         Assert.False(@true != new BOOL(true));
         Assert.True(@true != @false);
         Assert.False(@true == @false);
+
+        var two = new BOOL(2);
+        Assert.False(two == @true);
+        Assert.True(two != @true);
+    }
+
+    [Fact]
+    public void LogicalOperators_And()
+    {
+        BOOL @true = true, @false = false;
+        Assert.False(@false && @false);
+        Assert.False(@true && @false);
+        Assert.True(@true && @true);
+    }
+
+    [Fact]
+    public void LogicalOperators_Or()
+    {
+        BOOL @true = true, @false = false;
+        Assert.True(@true || @false);
+        Assert.False(@false || @false);
+        Assert.True(@true || @true);
     }
 }
diff --git a/test/GenerationSandbox.Tests/BooleanTests.cs b/test/GenerationSandbox.Tests/BooleanTests.cs
@@ -6,7 +6,7 @@
 public class BooleanTests
 {
     [Fact]
-    public void Boolean()
+    public void Ctor_Bool()
     {
         BOOLEAN b = true;
         bool b2 = b;
@@ -16,26 +16,31 @@ public void Boolean()
         Assert.False(default(BOOLEAN));
     }
 
-    [Theory]
-    [InlineData(3)]
-    [InlineData(0xff)]
-    public void NotLossyConversionBetweenBoolAndBOOLEAN(byte ordinal)
+    [Fact]
+    public void Ctor_byte()
     {
-        BOOLEAN nativeBool = new BOOLEAN(ordinal);
-        bool managedBool = nativeBool;
-        BOOLEAN roundtrippedNativeBool = managedBool;
-        Assert.Equal(nativeBool, roundtrippedNativeBool);
+        Assert.Equal(2, new BOOLEAN(2).Value);
+    }
+
+    [Fact]
+    public void ExplicitCast()
+    {
+        Assert.Equal(2, ((BOOLEAN)2).Value);
     }
 
     [Theory]
     [InlineData(3)]
     [InlineData(0xff)]
-    public void NotLossyConversionBetweenBoolAndBOOLEAN_Ctors(byte ordinal)
+    [InlineData(0x80)]
+    [InlineData(0x00)]
+    [InlineData(0x01)]
+    public void LossyConversionFromBOOLEANtoBool(byte ordinal)
     {
         BOOLEAN nativeBool = new BOOLEAN(ordinal);
         bool managedBool = nativeBool;
-        BOOLEAN roundtrippedNativeBool = new BOOLEAN(managedBool);
-        Assert.Equal(nativeBool, roundtrippedNativeBool);
+        Assert.Equal(ordinal != 0, managedBool);
+        BOOLEAN roundtrippedNativeBool = managedBool;
+        Assert.Equal(managedBool ? 1 : 0, roundtrippedNativeBool);
     }
 
     [Fact]
@@ -56,5 +61,27 @@ public void BOOLEAN_OverridesEqualityOperator()
         Assert.False(@true != new BOOLEAN(true));
         Assert.True(@true != @false);
         Assert.False(@true == @false);
+
+        var two = new BOOLEAN(2);
+        Assert.False(two == @true);
+        Assert.True(two != @true);
+    }
+
+    [Fact]
+    public void LogicalOperators_And()
+    {
+        BOOLEAN @true = true, @false = false;
+        Assert.False(@false && @false);
+        Assert.False(@true && @false);
+        Assert.True(@true && @true);
+    }
+
+    [Fact]
+    public void LogicalOperators_Or()
+    {
+        BOOLEAN @true = true, @false = false;
+        Assert.True(@true || @false);
+        Assert.False(@false || @false);
+        Assert.True(@true || @true);
     }
 }
diff --git a/test/Microsoft.Windows.CsWin32.Tests/GeneratorTests.cs b/test/Microsoft.Windows.CsWin32.Tests/GeneratorTests.cs
@@ -1632,14 +1632,8 @@ namespace Foundation
 			public override bool Equals(object obj) => obj is BOOL other && this.Equals(other);
 
 			public override int GetHashCode() => this.Value.GetHashCode();
-			internal unsafe BOOL(bool value) => this.Value = *(sbyte*)&value;
-			public static unsafe implicit operator bool(BOOL value)
-
-			{
-				sbyte v = checked((sbyte)value.Value);
-				return *(bool*)&v;
-			}
-
+			internal BOOL(bool value) => this.Value = value ? 1 : 0;
+			public static implicit operator bool(BOOL value) => value.Value != 0;
 			public static implicit operator BOOL(bool value) => new BOOL(value);
 		}
 	}
@@ -1904,14 +1898,8 @@ namespace Foundation
 			public override bool Equals(object obj) => obj is BOOL other && this.Equals(other);
 
 			public override int GetHashCode() => this.Value.GetHashCode();
-			internal unsafe BOOL(bool value) => this.Value = *(sbyte*)&value;
-			public static unsafe implicit operator bool(BOOL value)
-
-			{
-				sbyte v = checked((sbyte)value.Value);
-				return *(bool*)&v;
-			}
-
+			internal BOOL(bool value) => this.Value = value ? 1 : 0;
+			public static implicit operator bool(BOOL value) => value.Value != 0;
 			public static implicit operator BOOL(bool value) => new BOOL(value);
 		}
 	}
@@ -2231,14 +2219,8 @@ namespace Foundation
 			public override bool Equals(object obj) => obj is BOOL other && this.Equals(other);
 
 			public override int GetHashCode() => this.Value.GetHashCode();
-			internal unsafe BOOL(bool value) => this.Value = *(sbyte*)&value;
-			public static unsafe implicit operator bool(BOOL value)
-
-			{
-				sbyte v = checked((sbyte)value.Value);
-				return *(bool*)&v;
-			}
-
+			internal BOOL(bool value) => this.Value = value ? 1 : 0;
+			public static implicit operator bool(BOOL value) => value.Value != 0;
 			public static implicit operator BOOL(bool value) => new BOOL(value);
 		}
 	}