From 55f48679bc94a9803d36c6f3dfedf00bd601a5ea Mon Sep 17 00:00:00 2001 From: Andrew Arnott Date: Mon, 8 Aug 2022 21:33:18 -0600 Subject: [PATCH] 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 --- .../templates/BOOL.cs | 9 +--- .../templates/BOOLEAN.cs | 9 +--- test/GenerationSandbox.Tests/BoolTests.cs | 51 ++++++++++++++----- test/GenerationSandbox.Tests/BooleanTests.cs | 51 ++++++++++++++----- .../GeneratorTests.cs | 30 +++-------- 5 files changed, 88 insertions(+), 62 deletions(-) diff --git a/src/Microsoft.Windows.CsWin32/templates/BOOL.cs b/src/Microsoft.Windows.CsWin32/templates/BOOL.cs index 81711235..95cb4013 100644 --- 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 index 272cb05b..b23e3adf 100644 --- 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 index a4541914..3415091a 100644 --- 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 index 771b311b..2e3076a5 100644 --- 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 index 3b7c9866..fd524757 100644 --- a/test/Microsoft.Windows.CsWin32.Tests/GeneratorTests.cs +++ b/test/Microsoft.Windows.CsWin32.Tests/GeneratorTests.cs @@ -1632,14 +1632,8 @@ internal readonly partial struct BOOL 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 @@ internal readonly partial struct BOOL 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 @@ internal readonly partial struct BOOL 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); } }