Optimizing a conditional AND to a logical AND requires coercion

[hadibrais]

Version Used:
VS 2015

Description: Section III.1.1.2 of the Common Language Infrastructure (CLI) defines the System.Boolean type to be of size 1 byte where a bit pattern of all zeroes denotes False and any other bit pattern denotes True. Also it's a CLS type. On the hand, the C# specification states in 4.1.8 that the bool type is an alias to System.Boolean and represents two values, True and False, without saying anything about their implementation. This is all fine. The problem occurs when the C# compiler assumes that True is always represented as 0x01, which violates the CLS.

Steps to Reproduce: Consider the following method:

public static bool And(bool a, bool b)
{
        return a && b;
}

Expected Behavior: The compiler can either emit the code that implements the short-circuiting conditional AND or emits instructions to coerce both operands to the bit patterns 0x00 for False and 0x01 for True and use the logical (bitwise) AND CIL instruction.

Actual Behavior: The compiler emits the following instructions:

ldarg.0
ldarg.1
and

It applies the logical AND instruction on the arguments without coercing them. If both arguments were True with complementary bit patterns (which is CLS-compliant), the result would be False, which is incorrect.

Additional Information:

By very quickly going through the Roslyn code, I think that this transformation is implemented here, but I'm not sure. It seems that in Roslyn, the conditional AND is called the Logical AND and the Logical AND is called the Bool AND.

[svick]

Since doing this would likely make the code less efficient and since booleans with bit patterns other than 0x00 and 0x01 are extremely rare, maybe the CLI specification should be changed to say that only 0x01 is a valid value for True?

[svick]

Related: https://github.com/dotnet/roslyn/issues/12211

@gafter closed that issue with:

The spec doesn't say what is supposed to happen with corrupted bool values.

Though the issue does not mention the CLI spec.

[HaloFour]

As rare as it may be such behavior seems horribly incorrect. It doesn't matter how fast the IL is if the result is wrong.

[hadibrais]

I think that #12211 describes the same problem, but the reasoning is not solid. Boolean bit patterns other than 0 and 1 are not corrupted at all, they are valid and CLS-compliant. Correctness is infinitely more important than efficiency.

Changing the CLI spec is a valid option, but that's a massive breaking change. I prefer one of the two solutions presented in my original post. Somebody has got to measure the difference in perf and code size.

[AdamSpeight2008]

The value of 0 (Zero) is false and any other value is True, more specifically Not Zero. Which is typically the value of -1. And not as some think 1.

[svick]

Which is typically the value of -1. And not as some think 1.

Not really.

• Both C# and VB translate true to ldc.i4.1.
• C# does not allow casting bool to int.
• Casting Boolean to Integer in VB.NET using CType gives -1 for True.
• Convert.ToInt32(Boolean) returns 1 for true.
• Instructions like cgt return 1 for true.

So -1 is the outlier and VB.NET presumably uses it only for compatibility with VB6.

[HaloFour]

Which is typically the value of -1. And not as some think 1.

That was a relic specifically from the old days of BASIC where the values were calculated as follows:

FALSE = 0
TRUE = NOT FALSE

It was a horrible idea then it was a horrible idea when Microsoft continued it into VB. It was a continual source of very subtle bugs, although it usually only reared its ugly head when you had to perform Boolean login on 32-bit Long results from Win32 API.

The C# compiler is poised to repeat this same very awful mistake, except it looks like they intend to codify it directly into the language and bool datatype where it will be even more difficult to identify and rectify.

CLR/CLS defines true as non-zero. It would be the definition of wrong if the C# compiler considered it to be anything else.

[AdamSpeight2008]

@HaloFour @svick Where did I mention VB in my rationale?

It be rare of a modern cpu to have opcodes dedicate to byte operations, it is typically restricted to the native word length eg. 32bit or 64bit signed (2's complement ). Making the electronic logic circuits for the operators simpler to implement. Where detection of Zero (Boolean False) can be done in single gate XOR (Not Eor), thus not zero (Boolean True) is the complement of that.
Which also can (but not always) indicate = (equal to) and <> (not equal to)

            var xs = new List<int>()  { -2, -1, 0, 1, 2 };
            foreach(var x in xs)
            {
                bool b = false;
                b = Convert.ToBoolean(x);
                Console.WriteLine("{0,-5} {1,-5}", x,  b);
            }

outputs

-2    True
-1    True
0     False
1     True
2     True

[svick]

@AdamSpeight2008

Where did I mention VB in my rationale?

You didn't, but that's the only place I know that prefers -1 instead of 1 for true. And I wasn't disputing the first part of your statement (non-zero → true), only the second part (true → -1).

[AdamSpeight2008]

@svick I wonder what would happen if the opcode actually was ldc.i4.m1 rather than ldc.i4.1 ? for true.

[HaloFour]

@AdamSpeight

I didn't accuse you of mentioning it. I mentioned it because that was the justification for True being -1 in VB6 (and earlier), specifically to remain compatible with BASIC implementations from the 1970s. I don't doubt that said behavior was based on the simplification of logic gates in CPUs at the time. However, as a VB6 programmer who worked a great deal with Win32 API this was a very well known vector for bugs.

I'm sure that in most cases the C# compiler is doing the right thing, but it can't make assumptions when performing Boolean operations between bools. It doesn't matter whether or not being correct is less efficient. There is no non-zero epsilon for Boolean data types.

[AdamSpeight2008]

@svick @HaloFour Sorry I thought that you where.

[AdamSpeight2008]

What @hadibrais is saying.

 a = 1010 1010 (True according to CIL spec)
 b = 0101 0101 (True according to CIL spec)
     ---------  
 q = 0000 0000 AND (False accoring to CIL spec)

Which isn't correct according to boolean algebra. True And True -> True The coercion mention is the following. Which I think adds about 3-5 IL op (??) to the operators.

if( a != 0) a ==1;
if( b != 0) b==1;
return a && b;

[AdamSpeight2008]

Using Convert.ToBoolean the values are coerced to 0 and 1.

[AdamSpeight2008]

Sub Main

 Dim a1 = (New Corrupter With { .ByteValue = 125 }).BoolValue
 Dim b1 = (New Corrupter With { .ByteValue = 2 }).BoolValue
    Console.WriteLine(a1)
    Console.WriteLine(b1)
    Console.WriteLine()
    Dim z = _AndAlso_(a1,b1)
    Console.WriteLine(z)

End Sub

Function _AndAlso_(a As Boolean, b As Boolean) As Boolean
    'If( a <> 0) Then a =1
    'If( b <> 0) Then b=1
    Dim z= a AndAlso b
    Return z
End Function

' Define other methods and classes here
' Define other methods and classes here
<System.Runtime.InteropServices.StructLayout(System.Runtime.InteropServices.LayoutKind.Explicit)>
Structure Corrupter
    <System.Runtime.InteropServices.FieldOffset(0)>
    Public ByteValue As Byte
    <System.Runtime.InteropServices.FieldOffset(0)>
    Public BoolValue As Boolean
End Structure

produces

True
True

True

---

void Main()
{
 var a1 = new Corrupter { ByteValue = 125 }.BoolValue;
 var b1 = new Corrupter { ByteValue = 2 }.BoolValue;
    Console.WriteLine(a1);
    Console.WriteLine(b1);
        Console.WriteLine();
    var z = _AndAlso_(a1,b1);
    Console.WriteLine(z);
}
bool _AndAlso_(bool a, bool b)
{
  return a && b;
}

// Define other methods and classes here
[System.Runtime.InteropServices.StructLayout(System.Runtime.InteropServices.LayoutKind.Explicit)]
struct Corrupter
{
    [System.Runtime.InteropServices.FieldOffset(0)]
    public byte ByteValue;
    [System.Runtime.InteropServices.FieldOffset(0)]
    public bool BoolValue;
}

produces

True
True

False

---

IL for VB.net (via LINQPad)

IL_0000:  nop         
IL_0001:  ldloca.s    03 
IL_0003:  initobj     UserQuery.Corrupter
IL_0009:  ldloca.s    03 
IL_000B:  ldc.i4.s    7D 
IL_000D:  stfld       UserQuery+Corrupter.ByteValue
IL_0012:  ldloc.3     
IL_0013:  ldfld       UserQuery+Corrupter.BoolValue
IL_0018:  stloc.0     // a1
IL_0019:  ldloca.s    03 
IL_001B:  initobj     UserQuery.Corrupter
IL_0021:  ldloca.s    03 
IL_0023:  ldc.i4.2    
IL_0024:  stfld       UserQuery+Corrupter.ByteValue
IL_0029:  ldloc.3     
IL_002A:  ldfld       UserQuery+Corrupter.BoolValue
IL_002F:  stloc.1     // b1
IL_0030:  ldloc.0     // a1
IL_0031:  call        System.Console.WriteLine
IL_0036:  nop         
IL_0037:  ldloc.1     // b1
IL_0038:  call        System.Console.WriteLine
IL_003D:  nop         
IL_003E:  call        System.Console.WriteLine
IL_0043:  nop         
IL_0044:  ldarg.0     
IL_0045:  ldloc.0     // a1
IL_0046:  ldloc.1     // b1
IL_0047:  call        UserQuery._AndAlso_
IL_004C:  stloc.2     // z
IL_004D:  ldloc.2     // z
IL_004E:  call        System.Console.WriteLine
IL_0053:  nop         
IL_0054:  ret         

RunUserAuthoredQuery:
IL_0000:  nop         
IL_0001:  ldarg.0     
IL_0002:  call        UserQuery.Main
IL_0007:  nop         
IL_0008:  ret         

_AndAlso_:
IL_0000:  nop         
IL_0001:  ldarg.1     
IL_0002:  brfalse.s   IL_0007
IL_0004:  ldarg.2     
IL_0005:  br.s        IL_0008
IL_0007:  ldc.i4.0    
IL_0008:  stloc.1     // z
IL_0009:  ldloc.1     // z
IL_000A:  stloc.0     // _AndAlso_
IL_000B:  br.s        IL_000D
IL_000D:  ldloc.0     // _AndAlso_
IL_000E:  ret         

MyApplication..ctor:
IL_0000:  ldarg.0     
IL_0001:  call        Microsoft.VisualBasic.ApplicationServices.ConsoleApplicationBase..ctor
IL_0006:  ret         

MyComputer..ctor:
IL_0000:  nop         
IL_0001:  ldarg.0     
IL_0002:  call        Microsoft.VisualBasic.Devices.Computer..ctor
IL_0007:  nop         
IL_0008:  ret         

MyProject.get_Computer:
IL_0000:  nop         
IL_0001:  ldsfld      My.MyProject.m_ComputerObjectProvider
IL_0006:  callvirt    My.MyProject+ThreadSafeObjectProvider<My.MyComputer>.get_GetInstance
IL_000B:  stloc.0     // Computer
IL_000C:  br.s        IL_000E
IL_000E:  ldloc.0     // Computer
IL_000F:  ret         

MyProject.get_Application:
IL_0000:  nop         
IL_0001:  ldsfld      My.MyProject.m_AppObjectProvider
IL_0006:  callvirt    My.MyProject+ThreadSafeObjectProvider<My.MyApplication>.get_GetInstance
IL_000B:  stloc.0     // Application
IL_000C:  br.s        IL_000E
IL_000E:  ldloc.0     // Application
IL_000F:  ret         

MyProject.get_User:
IL_0000:  nop         
IL_0001:  ldsfld      My.MyProject.m_UserObjectProvider
IL_0006:  callvirt    My.MyProject+ThreadSafeObjectProvider<Microsoft.VisualBasic.ApplicationServices.User>.get_GetInstance
IL_000B:  stloc.0     // User
IL_000C:  br.s        IL_000E
IL_000E:  ldloc.0     // User
IL_000F:  ret         

MyProject.get_WebServices:
IL_0000:  nop         
IL_0001:  ldsfld      My.MyProject.m_MyWebServicesObjectProvider
IL_0006:  callvirt    My.MyProject+ThreadSafeObjectProvider<My.MyProject+MyWebServices>.get_GetInstance
IL_000B:  stloc.0     // WebServices
IL_000C:  br.s        IL_000E
IL_000E:  ldloc.0     // WebServices
IL_000F:  ret         

MyProject..cctor:
IL_0000:  newobj      My.MyProject+ThreadSafeObjectProvider<My.MyComputer>..ctor
IL_0005:  stsfld      My.MyProject.m_ComputerObjectProvider
IL_000A:  newobj      My.MyProject+ThreadSafeObjectProvider<My.MyApplication>..ctor
IL_000F:  stsfld      My.MyProject.m_AppObjectProvider
IL_0014:  newobj      My.MyProject+ThreadSafeObjectProvider<Microsoft.VisualBasic.ApplicationServices.User>..ctor
IL_0019:  stsfld      My.MyProject.m_UserObjectProvider
IL_001E:  newobj      My.MyProject+ThreadSafeObjectProvider<My.MyProject+MyWebServices>..ctor
IL_0023:  stsfld      My.MyProject.m_MyWebServicesObjectProvider
IL_0028:  ret         

MyWebServices.Equals:
IL_0000:  nop         
IL_0001:  ldarg.0     
IL_0002:  ldarg.1     
IL_0003:  call        System.Runtime.CompilerServices.RuntimeHelpers.GetObjectValue
IL_0008:  call        System.Object.Equals
IL_000D:  stloc.0     // Equals
IL_000E:  br.s        IL_0010
IL_0010:  ldloc.0     // Equals
IL_0011:  ret         

MyWebServices.GetHashCode:
IL_0000:  nop         
IL_0001:  ldarg.0     
IL_0002:  call        System.Object.GetHashCode
IL_0007:  stloc.0     // GetHashCode
IL_0008:  br.s        IL_000A
IL_000A:  ldloc.0     // GetHashCode
IL_000B:  ret         

MyWebServices.GetType:
IL_0000:  nop         
IL_0001:  ldtoken     My.MyProject.MyWebServices
IL_0006:  call        System.Type.GetTypeFromHandle
IL_000B:  stloc.0     // GetType
IL_000C:  br.s        IL_000E
IL_000E:  ldloc.0     // GetType
IL_000F:  ret         

MyWebServices.ToString:
IL_0000:  nop         
IL_0001:  ldarg.0     
IL_0002:  call        System.Object.ToString
IL_0007:  stloc.0     // ToString
IL_0008:  br.s        IL_000A
IL_000A:  ldloc.0     // ToString
IL_000B:  ret         

MyWebServices.Create__Instance__:
IL_0000:  nop         
IL_0001:  ldarg.0     
IL_0002:  box         My.MyProject+MyWebServices.T
IL_0007:  ldnull      
IL_0008:  ceq         
IL_000A:  stloc.1     
IL_000B:  ldloc.1     
IL_000C:  brfalse.s   IL_0016
IL_000E:  call        System.Activator.CreateInstance<T>
IL_0013:  stloc.0     // Create__Instance__
IL_0014:  br.s        IL_001B
IL_0016:  nop         
IL_0017:  ldarg.0     
IL_0018:  stloc.0     // Create__Instance__
IL_0019:  br.s        IL_001B
IL_001B:  ldloc.0     // Create__Instance__
IL_001C:  ret         

MyWebServices.Dispose__Instance__:
IL_0000:  nop         
IL_0001:  ldarg.1     
IL_0002:  initobj     My.MyProject+MyWebServices.T
IL_0008:  ret         

MyWebServices..ctor:
IL_0000:  nop         
IL_0001:  ldarg.0     
IL_0002:  call        System.Object..ctor
IL_0007:  nop         
IL_0008:  ret         

ThreadSafeObjectProvider`1.get_GetInstance:
IL_0000:  nop         
IL_0001:  ldsfld      My.MyProject+ThreadSafeObjectProvider<>.m_ThreadStaticValue
IL_0006:  box         My.MyProject+ThreadSafeObjectProvider<>.T
IL_000B:  ldnull      
IL_000C:  ceq         
IL_000E:  stloc.1     
IL_000F:  ldloc.1     
IL_0010:  brfalse.s   IL_001C
IL_0012:  call        System.Activator.CreateInstance<T>
IL_0017:  stsfld      My.MyProject+ThreadSafeObjectProvider<>.m_ThreadStaticValue
IL_001C:  ldsfld      My.MyProject+ThreadSafeObjectProvider<>.m_ThreadStaticValue
IL_0021:  stloc.0     // GetInstance
IL_0022:  br.s        IL_0024
IL_0024:  ldloc.0     // GetInstance
IL_0025:  ret         

ThreadSafeObjectProvider`1..ctor:
IL_0000:  nop         
IL_0001:  ldarg.0     
IL_0002:  call        System.Object..ctor
IL_0007:  nop         
IL_0008:  ret         

IL for C# (via LINQPad)

IL_0000:  nop         
IL_0001:  ldloca.s    03 
IL_0003:  initobj     UserQuery.Corrupter
IL_0009:  ldloca.s    03 
IL_000B:  ldc.i4.s    7D 
IL_000D:  stfld       UserQuery+Corrupter.ByteValue
IL_0012:  ldloc.3     
IL_0013:  ldfld       UserQuery+Corrupter.BoolValue
IL_0018:  stloc.0     // a1
IL_0019:  ldloca.s    03 
IL_001B:  initobj     UserQuery.Corrupter
IL_0021:  ldloca.s    03 
IL_0023:  ldc.i4.2    
IL_0024:  stfld       UserQuery+Corrupter.ByteValue
IL_0029:  ldloc.3     
IL_002A:  ldfld       UserQuery+Corrupter.BoolValue
IL_002F:  stloc.1     // b1
IL_0030:  ldloc.0     // a1
IL_0031:  call        System.Console.WriteLine
IL_0036:  nop         
IL_0037:  ldloc.1     // b1
IL_0038:  call        System.Console.WriteLine
IL_003D:  nop         
IL_003E:  call        System.Console.WriteLine
IL_0043:  nop         
IL_0044:  ldarg.0     
IL_0045:  ldloc.0     // a1
IL_0046:  ldloc.1     // b1
IL_0047:  call        UserQuery._AndAlso_
IL_004C:  stloc.2     // z
IL_004D:  ldloc.2     // z
IL_004E:  call        System.Console.WriteLine
IL_0053:  nop         
IL_0054:  ret         

_AndAlso_:
IL_0000:  nop         
IL_0001:  ldarg.1     
IL_0002:  ldarg.2     
IL_0003:  and         
IL_0004:  stloc.0     
IL_0005:  br.s        IL_0007
IL_0007:  ldloc.0     
IL_0008:  ret         

---

So it seem to me that C# is producing the incorrect IL for the && operation. C#

IL_0000:  nop         
IL_0001:  ldarg.1     
IL_0002:  ldarg.2     
IL_0003:  and         
IL_0004:  stloc.0     
IL_0005:  br.s        IL_0007
IL_0007:  ldloc.0     
IL_0008:  ret   

vs VB.net's

IL_0000:  nop         
IL_0001:  ldarg.1     
IL_0002:  brfalse.s   IL_0007
IL_0004:  ldarg.2     
IL_0005:  br.s        IL_0008
IL_0007:  ldc.i4.0    
IL_0008:  stloc.1     // z
IL_0009:  ldloc.1     // z
IL_000A:  stloc.0     // _AndAlso_
IL_000B:  br.s        IL_000D
IL_000D:  ldloc.0     // _AndAlso_
IL_000E:  ret 

[mikedn]

While it is true that this looks like a gratuitous change in the Roslyn compiler invoking III.1.1.2 from ECMA spec doesn't help a lot. If we agree that non-zero means true then we'd have to change pretty much all other bool operators - ==, !=, &, | and ^. The only one that works "correctly" currently is !. As long as these operators continue to work as before (and they work like this since day one) you can't reliably use bool values where true may be represented by a non-zero value other than 1. From this perspective the Roslyn change is no longer gratuitous, it simply went down the path that was set a long time ago.

Besides, what III.1.1.2 says should be taken with a grain of salt. It's quite possible that someone thought that "well, brtrue jumps if the argument is non-zero so let's say that non-zero means true". But there's nothing else that suggests that a lot of consideration went into this. IL doesn't actually know about bools, it only deals with integers. IL instructions that are commonly used to produce bools (ceq, clt etc.) always produce 0/1, not 0/non-0.

The ECMA spec should specify which value(s) are treated as true but in the end languages that may wish to produce something other than 1 for true need to go out of their way to do so exactly because IL instructions produce ones. Ultimately you end up in a situation where what the ECMA spec currently says is kind of pointless, you need more code to produce and consume such bools and the only people who could benefit from this are those who accidently produce such bools. Or intentionally but if you do this intentionally then you're basically asking for trouble.

[HaloFour]

@mikedn

It doesn't matter what someone really meant. What matters is what the spec says. The C# spec defers to the CLS spec which has long since established the valid values for a bool. By not accepting and handling those values appropriately C# becomes no longer a CLS language. The C# compiler can't rely on what Microsoft internally decided passes as true, it needs to be a good citizen with the several dozen other CLR-based languages. It already appears that C# and VB.NET don't agree.

Just to confirm, is this a "regression" with the Roslyn-based C# compilers or has C# always treated bools like this? I don't have an older version of VS available to test.

[hadibrais]

@mikedn

If we agree that non-zero means true then we'd have to change pretty much all other bool operators - ==, !=, &, | and ^.

I agree that resolving this issue would require a non-trivial amount of work.

As long as these operators continue to work as before (and they work like this since day one) you can't reliably use bool values where true may be represented by a non-zero value other than 1. From this perspective the Roslyn change is no longer gratuitous, it simply went down the path that was set a long time ago.

There is no problem that the C# compiler has always implemented the bool operators under the assumption that True has the bit pattern 0x01. The problem occurs when the C# spec claims that bool is an alias for System.Boolean because the assumption does not hold in this case.

But there's nothing else that suggests that a lot of consideration went into this.

I don't agree. At the time the first version of CLI were written, C, C++, Java and JavaScript were commonly used languages in which converting a zero value to the boolean type produces False and converting any non-zero value to the boolean type produces True. (I believe that the original motivation for this was to support the C/C++ expression if(ptr) to check for null pointers.) Most likely CLI followed the same convention.

IL doesn't actually know about bools, it only deals with integers. IL instructions that are commonly used to produce bools (ceq, clt etc.) always produce 0/1, not 0/non-0.

While this is true, consider also that these instructions consume bools that are 0/non-0, not 0/1. The JIT compiler has been implemented accordingly. The C# compiler has been implemented based on an invalid assumption. Also the reason that they produce 0/1 is historical (that what happened in other languages).

Or intentionally but if you do this intentionally then you're basically asking for trouble.

How does following the spec mean that the programmer is asking for trouble?

[mikedn]

What matters is what the spec says

C# hasn't followed the spec in this regard for 15 years now. What the spec says is now irelevant.

It already appears that C# and VB.NET don't agree.

They actually do, neither is following the spec. Nor does C++/CLI.

The problem occurs when the C# spec claims that bool is an alias for System.Boolean because the assumption does not hold in this case.

If there's a disagreement between specs and implementations then either the implementation or the spec has issues. Or both. Considering that there are multiple implementations that disagree with the spec but more or less agree with each other and considering that following the spec imposes a performance penalty that's not justified given the rarity of non-one trues it is perfectly reasonably to treat the spec as broken and fix it or ignore it.

At the time the first version of CLI were written, C, C++, Java and JavaScript were commonly used languages in which converting a zero value to the boolean type produces False and converting any non-zero value to the boolean type produces True. (I believe that the original motivation for this was to support the C/C++ expression if(ptr) to check for null pointers

I can't speak of Java and JavaScript but what you say is certainly incorrect in the case of C/C++. You are confusing conversions between bool and integral types and bool representations. Interestingly enough you're not the only one in this thread making such a mistake.

While this is true, consider also that they these instructions consume bools that are 0/non-0, not 0/1. The JIT compiler has been implemented accordingly.

There are no such instructions and bools do not exist on the evaluation stack. The JIT compiler has nothing to do with this.

How is following the spec mean that the programmer is asking for trouble?

It doesn't matter what the spec says. What matters is that many (maybe all) implementations produce 0/1 bools. That means that there's a good chance that some implementation can screw up and fail to work with 0/non-zero bools. Producing 0/non-zero bools means higher risk and buys you nothing. That's asking for trouble.

[AdamSpeight2008]

But it should have the same results as VB.net, consider that the emitter for that code looks identical to me. If it is, then it should produce the same result. Also consider the case where the both side have side-effects, like incrementing a common value. If first evaluated to False, the final expected common would be +1, if it +2 then it has evaluated both sides. Which is incorrect according to the semantics of short-circuiting and &&

[hadibrais]

You are confusing conversions between bool and integral types and bool representations.

I carefully used the term "converting" to indicate that I am referring to conversions, not representations.

There are no such instructions and bools do not exist on the evaluation stack. The JIT compiler has nothing to do with this.

I agree that the JIT compiler has nothing to do this.

following the spec imposes a performance penalty that's not justified given the rarity of non-one trues it is perfectly reasonably to treat the spec as broken and fix it or ignore it.

The C# compiler can quickly perform some simple static analysis to determine whether a bool can have a value larger than one. There are three contexts in which the compiler can conservatively decide that a bool can have any value: when it's field that overlaps with some other field using FieldOffset, in an unsafe block or method, and when the bool value may come from another DLL that may have been written in another language. In all other contexts, the compiler can safely assume that the bool is 0/1 and perform the applicable optimizations. This way you get to keep the spec and the optimization. This is just one option that I think is wroth considering. You've also suggested two other options: either consider the spec as broken and fix it or ignore it. But if you ignore it, it's going to bite some people. Fixing the spec is the easiest option.

[mikedn]

I carefully used the term "converting" to indicate that I am referring to conversions, not representations.

And this issue has to do with bool representations, it has nothing to do with conversions. The whole C/C++ story doesn't belong here.

The C# compiler can quickly perform some simple static analysis

There's no such thing as "simple" static analysis. What happens if your example And function is called from an unsafe context? Will it treat the bools as 0/1 or as 0/non-zero? What should happen if it is called from both safe and unsafe contexts? What should happen if [FieldOffset] fields are passed by ref? How do you know that a bool came from another DLL?

[hadibrais]

Yes, this issue has to do with representations not conversions. The reason I was discussing conversions is to reasonably guess why the CLI spec was written like this in the first place.

Regarding the analysis, I'm suggesting a conservative analysis. In all the cases you mentioned, the compiler treats the bools as 0/non-0. Of course, this reduces the ability of the compiler to perform bool optimizations. This analysis can be more complicated to make it more accurate.

How do you know that a bool came from another DLL?

It's sufficient to determine whether a bool MAY come from another DLL. By checking the accessibility of the type and the type member of interest, we can determine whether the value of the Boolean field/variable/parameter may come from another DLL.

[Joe4evr]

what you say is certainly incorrect in the case of C/C++.

Historically, C doesn't even have bools.

[DavidArno]

@Joe4evr, ah those were the days, when folk would #define TRUE = 0, so that a successful app could exit with TRUE :scream:

[SunnyWar]

@mikedn

C# hasn't followed the spec in this regard for 15 years now. What the spec says is now irelevant.

I had to deal with a very hard to track down bug a couple months ago that was caused by the developer going against the current spec with regards to this exact operation. In short, I do not think this is a bug in the spec, it's a bug in .Net.

Simply: it should be fixed.

[hadibrais]

@SunnyWar Agreed. Most people follow what the spec says, not what the source code does. This sounds like a 15-year old bug in the C# compiler. As @AdamSpeight2008 has pointed out, this is only a C# compiler issue. So we've got only one thing to fix. Changing the CLI spec just because the C# compiler has been implemented in this way sounds like a radical fix. Not to mention the subtle breaking change that this fix would introduce.

[mikedn]

I'm suggesting a conservative analysis

It's likely to end up being so conservative that you may as well make the compile follow the spec in all circumstances.

By checking the accessibility of the type and the type member of interest

Accessibility cannot be relied upon due to reflection.

it's a bug in .Net.

You mean in the C# compiler. And in the VB.NET compiler. And in the C++/CLI compiler. And who knows how many others. Good luck fixing that. And even if you fix compilers it's still possible to run into bugs caused by disagreements over the representation of bools. Someone can produce a method like And by using LINQ expressions thinking that bool is 0/1. Someone could get a non-1 true from somewhere and pass it to native code that expects 1 not realizing that true is not always 1.

As @AdamSpeight2008 has pointed out, this is only a C# compiler issue. So we've got only one thing to fix.

I already stated that this is not a C# only issue. There are multiple compilers and multiple operators that don't follow the spec.

[hadibrais]

Accessibility cannot be relied upon due to reflection.

Indeed. I missed that.

I already stated that this is not a C# only issue. There are multiple compilers and multiple operators that don't follow the spec.

Are all of these compilers consistent with each other but only disagree with the spec? Are you going to change the CLI spec so that True/False are represented as 1/0 only? Does this change in spec trigger any further changes in any of the compilers?

[mikedn]

Are all of these compilers consistent with each other but only disagree with the spec?

They're consistent with the exception of the logical ops (&& and ||). This includes VC++'s bitwise ops which according to the C++ standard require conversion from bool to int. VC++ assumes that bool is 0/1 so it skips the conversion. And native C++ compilers like GCC and Clang do the same thing.

Are you going to change the CLI spec so that True/False are represented as 1/0 only?

Me? I don't work for Microsoft.

Does this change in spec trigger any further changes in any of the compilers?

Compilers that somehow produce non-1 trues would have to be changed. Anyone knows such a compiler?

[hadibrais]

They're consistent with the exception of the logical ops (&& and ||).

I guess you're referring to the inconsistency between VB and C# described above. The VB compiler doesn't perform the optimization mentioned in my original post. I wonder why the C# compiler supported the optimization for 15 years, but not the VB compiler. Maybe the VB team intended to follow the spec back then.

[mikedn]

I wonder why the C# compiler supported the optimization for 15 years

It didn't, I said in my first post that this is a Roslyn change. But 15 years ago it did implement other bool operators by assuming 0/1 bools. And so did VB.NET. I don't have VS 2002.NET to check what C++/CLI did back then but considering native VC++ always assumed 0/1 bools it's unlikely that C++/CLI did something else.

BTW, System.Boolean's own Equals doesn't handle 0/non-zero bools. Shouldn't be surprising considering that it is implemented in C# and uses ==.

[hadibrais]

Hopefully somebody from Microsoft will see this issue and do something about it.

[jaredpar]

While the CLR allows for a bool value to have the full range of a byte, the C# language only recognizes that a bool can have true and false. Or more specifically 1 and 0. The compiler generates code according to that assumption.

Usually this isn't a problem because it's actually fairly hard to get a corrupted bool value in C#. Most of the major entry points for this are guarded: pinvoke, coercion from numeric types, etc ... The only way I've really found to do it is direct bit manipulation via explicit struct layouts. For example:

class Program
{
    [StructLayout(LayoutKind.Explicit)]
    struct Union
    {
        [FieldOffset(0)]
        internal byte ByteField;

        [FieldOffset(0)]
        internal bool BoolField;
    }

    static void Main(string[] args)
    {
        Union u1 = new Union();
        Union u2 = new Union();
        u1.ByteField = 1;
        u2.ByteField = 2;
        Console.WriteLine(u1.BoolField); // True
        Console.WriteLine(u2.BoolField); // True
        Console.WriteLine(u1.BoolField == u2.BoolField); // False
    }
}

In general though the compiler views behavior like this as By Design. The language only recognizes bool as 1 or 0 and operates accordingly.

[VSadov]

This was discussed more than once in the past.

For a very detailed discussion take a look at https://roslyn.codeplex.com/workitem/224

TL;DR version:

C# spec specifies bool as having just 2 states - true and false and does not prescribe any particular implementation. The implementation chosen on .Net CLI maps bool to i8 in storage and i32 in computations, while only 0 and 1 values are used. – those are the values of false/true literals and also what comparison operators produce. If through some bit-fiddling you've managed to make a bool with a value 42, you broke it. You will have one “true” not equal to another “true”, do not be surprised about the rest of the mess.

[HaloFour]

@jaredpar @VSadov

If that is to remain the case I think that the C# spec should very explicitly state this, and the CLS spec should be updated to vehemently discourage other values. The C# spec only mentions that the valid values are true and false but does nothing to define what either means, or that true means something more specific than "non-zero".

I still think this is a mistake simply because CLS states quite clearly that anything non-zero is true and there's no reason for a third-party language designer to decide to use, say, ldc.i4.m1 for true or to allow basic arithmetic operations to be treated implicitly as Boolean values. If I were writing a "BASIC.NET" then I would absolutely use ldc.i4.m1, and I would be making the massive undocumented error of assuming that my language would be CLS-compliant and interoperate appropriately with other so-called CLS-compliant languages.

[VSadov]

@HaloFour C# spec really has no way to state this. Would it say "do not assign bool variables any values other than true and false"? It already implies that.

Ideally bool would be implemented on top of a 1 bit primitive. Such thing does not exist in CLI. The smallest chunk of storage is 8bit. And the smallest integral that can participate in computations is 32bit. While in conditional branch operators (like brtrue) any nonzero value is considered not-false, the canonical value of true (as produced by ceq instruction) is 1. This forces any efficient implementation to be like what we have in C#.

"BASIC.NET" would need to jump through some major hoops when implementing vbBool since platform is clearly optimized for 1 and 0.

IMO. If there are any docs saying that bool values other than 1 and 0 are ok for universal interop purposes, they are misleading and need changing.

[hadibrais]

IMO there is nothing wrong with the C# spec regarding this issue. The problem occurred when the Visual C# compiler was implemented consistently with respect to the C# spec, but inconsistently with the CLI.

This forces any efficient implementation to be like what we have in C#.

Are you referring to the optimizations such as the one described in my original post? (those that depend on true being 1). Were these implemented in the first version of the compiler?

To make all of these optimizations CLI-conformant and CLS-compliant, the CLI spec should be changed so that System.Boolean is defined to be of size 1 byte where a bit pattern of all zeroes denotes False, a bit patter of 0x1 denotes True, and any other bit pattern results in undefined behavior.

In this case, the compiler of "BASIC.NET" would have to coerce (normalize) bools before storing them in a location of type System.Boolean.

[Joe4evr]

The only way I've really found to do it is direct bit manipulation via explicit struct layouts.

Yes, @AdamSpeight2008 already posted the exact same thing, with an in-depth analysis. But this whole issue is about the fact that given such corrupted bool values, u1.BoolField && u2.BoolField behaves like u1.BoolField & u2.BoolField (note one less ampersand) and so potentially evaluates to false (if neither has a same bit set) even though both of them individually evaluate to true. And on the other hand, VB's AndAlso doesn't do this and and handles the corrupted values "properly".

[gafter]

I labeled this a "Bug" because C#, the principal language on the CLI platform, doesn't comply with the CLI specification on the meaning of the underlying represenation of the boolean type. Since the CLI spec should lead the interpretation of CLI data structures for compilers that intend to comply with the Common Language Infrastructure standard, this is technically a bug.

Having said that, we do get a performance advantage from this treatment that we are loathe to lose. I expect we are likely to "fix" this bug by documenting the deviation from the CLI specification.

[VSadov]

CLI literally says the following:

I II.1.1.2 Boolean data type A CLI Boolean type occupies 1 byte in memory. A bit pattern of all zeroes denotes a value of false. A bit pattern with any one or more bits set (analogous to a non-zero integer) denotes a value of true. For the purpose of stack operations boolean values are treated as unsigned 1-byte integers

The requirement "any one or more bits set" is sufficient for CLI since it does not define logical operators for bool values. The only operations, that are consuming bool operands are conditional operators (like brtrue), and bitwise operations on the underlying integer (and, or). In either case uniqueness of "true" values is not a requirement for CLI to function.

C#, however, implements boolean operations and those rely on bool being two-state. So compiler should either normalize all bool operands (expensive) or assume that they have canonical values (not easy, but possible to break as we have seen here).

Yes, while CLI internally does not require two-state bool, most languages on top would require/benefit from two-state bool.

Documenting this assumption, at least as an interop requirement with C#/VB, seems like a good idea.

[mikedn]

the purpose of stack operations boolean values are treated as unsigned 1-byte integers

Note that the CLI spec is wrong/confusing here because the stack only operates on 4/8 bytes integers.

Also, the fact that they're unsigned integers will lead to problems if a language is trying to use 0/-1 bools. A -1 bool loaded on the evaluation stack will become 0xFF and after a not it will become 0xFFFFFF00 and not 0. And of course, passing 0xFFFFFF00 to brtrue is not the same thing as passing 0. Cheap negation would be the only reason to use 0/-1 but then it turns out that it's not really cheap due to CLI bools being unsigned.

[hadibrais]

Documenting this assumption, at least as an interop requirement with C#/VB, seems like a good idea.

How is this better than "fixing" the CLI spec? If we do this, C# would not longer be CLS-compliant and .NET will no longer be conformant with the CLI. By "fixing" the CLI spec, we can "preserve" these features.

Note that the CLI spec is wrong/confusing here because the stack only operates on 4/8 bytes integers.

Well, the definition of Boolean should either state that the 1-byte is treated as unsigned or signed. Otherwise, it would be ambiguous. The part you referred is the only place where it says that it's treated as an unsigned byte. So it's necessary. I don't know whether there is a reason for choosing the unsigned treatment. You've already given a reason to favor the signed treatment.

[mikedn]

Well, the definition of Boolean should either state that the 1-byte is treated as unsigned or signed. Otherwise, it would be ambiguous.

Yes, most likely that part refers to load/store instructions since those are the only ones that deal with actual bools. BTW, the size discrepancy between memory and stack is another potential source of problems for 0/non-zero bools. If you pass 0x10000000 to brtrue it will be treated as true but when you store that value in a bool field it will get truncated to 0 and become false.

[hadibrais]

In addition to what @mikedn said, there is another advantage of using 0/-1 bools and signed extension for the evaluation stack. Consider the following expression:

a = x ? a : 0;

Currently, the compiler has to emit a conditional branch and and the corresponding blocks in the IL code. However, assuming 0/-1 bools and signed extension, the expression can be rewritten as:

a = x & a;

This does not require conditional logic in the IL. The IL would consist of loading a bool on the stack, loading a value on the stacking, ANDing them and finaly storing the result.

I believe the JIT compiler can easily identify this pattern and emit a very efficient machine code for it by either using a comparison followed by CMOV or an AND instruction. This is faster than the code that the JIT currently generates because of the conditional jump.

This also applies to code of the form:

a = 0; b = 0; g = 0; if(x) { a = c1; b = c2; g = c3; }

The C# compiler can emit IL code as if it has been written like:

a = c1; a = x ? a : 0; b = c2; b = x ? b : 0; g = c3; g = x ? g : 0;

The JIT can then easily identify the pattern.

[mikedn]

there is another advantage of using 0/-1 bools and signed extension for the evaluation stack. Consider the following expression:

Yeah but 0/-1 bools have a disadvantage - to produce them you need an extra instruction in both IL and assembly code (at least on x86/x64 which has setcc that produces 0/1). Anyway, since 0/1 has been the de facto standard for a long time it's very unlikely that we'll now change from 0/1 to 0/-1 no matter what advantages 0/-1 may have.

I believe the JIT compiler can easily identify this pattern and emit a very efficient machine code for it by either using a comparison followed by CMOV or an AND instruction.

Yes, it's possible to identify such patterns and there are issues opened for that. Though I doubt the JIT will ever recognize an if as large as the one you show, it will more likely be limited to one assignment per block. But that's another story.