Char concat

Hi!

I have a bug with G120, latest firmware.

var s = "11";
s += ' '; // space
// now s = "1132"

So, space char is converted to byte while concatenating.

1 Like

Try in the emulator please.

that’s interesting. string += char. = string “1132”.

I don’t get this bug on my CerbBee or in the emulator.

Absolutely! I ported my project from Cerb to G120 and got that bug. Reproduced only on G120.

The code in the first post is simplified. The actual code is located here:

 data, Char spacer)
        {
            String s = "";
            foreach (byte b in data)
            {
                if (s.Length > 0)
                {
                    s += spacer;
                }
                s += b.ToHex();
            }
            return s;
        }

I ran the following code based on the code in your source repo:

using System;
using Microsoft.SPOT;

namespace hextest
{
    public class Program
    {
        public static void Main()
        {
            byte[] foo = new byte[] { 0x11, 0x22, 0x33, 0x44 };
            Debug.Print(foo.ToHex(' '));
        }
    }

    public static class Extensions
    {
        const string hexChars = "0123456789ABCDEF";

        public static String ToHex(this byte b)
        {
            return hexChars[b >> 4].ToString() + hexChars[b & 0x0F].ToString();
        }

        public static String ToHex(this byte[] data, Char spacer)
        {
            String s = "";
            foreach (byte b in data)
            {
                if (s.Length > 0)
                {
                    s += spacer;
                }
                s += b.ToHex();
            }
            return s;
        }
    }
}

And I get the same error that you get. However, if I change


to

```cs]s += spacer.ToString();[/code


then it works correctly.  I debugged through it with the framework source code and 'spacer' is being implicitly typed as Int32 and then coerced to string via Int32.ToString(). By using .ToString(), this runs an explicit conversion to string first, which avoids the default conversion to Int32. This seems to be an issue in the compiler with the type presumptions and the IL that it is generating, and not in the framework. I get the same results in the emulator, and the framework code seems right - it's just making odd choices as to which framework code to call.

Overall, the code you provided will cause a lot of heap churn through repetitive re-allocation of non-mutable strings and it should be re-written to use StringBuilder. That will run faster and result in less heap fragmentation.
1 Like

Interesting - VS2013 returns the expected result. VS2015 returns the counter-intuitive result. So it is not related to the framework version. It is a change in the compiler. This is something everybody here should be aware of as it changes some pretty fundamental type-system behavior.

These two VS versions are running different compilers, so it seems that the new (Roslyn) compiler is making different type-coercion choices, and in this case, this seems like a big breaking change, and not necessarily a good one. Seems like a bug to me. I haven’t explored whether it is limited to generation of NETMF IL or all IL targets.

EDIT: Win32 C# works as expected under VS2013 and VS2015. This type-coercion weirdness seems to only affect NETMF.
EDIT2 : Filed as issue Incorrect coercion of char to Int32 · Issue #327 · NETMF/netmf-interpreter · GitHub

2 Likes

@ mcalsyn - Good job, thank you!

About, StringBuilder, yes, I know, thank you! :slight_smile: There are many not optimized methods to avoid “braking” some code in different NETMF versions starting from 4.1. I hope, I’ll optimize them later when run out of resources of Cerb.

@ toxsedyshev - When you last tested it on the Cerberus, was it in VS2013 or VS2015? I tested the G30, G80, G120, G400, and Cerberus. All five worked fine in VS2013 while all five failed with your bug in VS2015.

Interestingly, it was only the code @ mcalsyn posted that failed. If I ran the below code, VS2013 and VS2015 both gave the expected result.


public class Program {
	public static void Main() {
		var s = "11";
		s += ' ';
	}
}

@ John - Char constants appear to be getting treated differently than char-typed vars then, because that’s the main difference in your snippet.

As I remember, is was VS2013 with Cerb. So, it proves mcalsyn’s investigation about different compiler used in VS2015.

This is the Roslyn compiler for VS2015?

@ Mr. John Smith - yes

Good read…
Also see the comments.

http://blogs.msdn.com/b/vcblog/archive/2015/09/25/rejuvenating-the-microsoft-c-c-compiler.aspx?PageIndex=5#comments

Just in case you haven’t geeked out enough on this question, here’s the IL difference between the two compilers…

VS2013 NETMF

.method public hidebysig static void  Main() cil managed
{
  .entrypoint
  // Code size       24 (0x18)
  .maxstack  2
  .locals init ([0] string s,
           [1] char c)
  IL_0000:  nop
  IL_0001:  ldstr      "11"
  IL_0006:  stloc.0
  IL_0007:  ldc.i4.s   32
  IL_0009:  stloc.1
  IL_000a:  ldloc.0
  IL_000b:  ldloc.1
  IL_000c:  box        [mscorlib]System.Char
  IL_0011:  call       string [mscorlib]System.String::Concat(object,
                                                              object)
  IL_0016:  stloc.0
  IL_0017:  ret
} // end of method Program::Main

VS2015 NETMF

.method public hidebysig static void  Main() cil managed
{
  .entrypoint
  // Code size       31 (0x1f)
  .maxstack  2
  .locals init ([0] string s,
           [1] char c)
  IL_0000:  nop
  IL_0001:  ldstr      "11"
  IL_0006:  stloc.0
  IL_0007:  ldc.i4.s   32
  IL_0009:  stloc.1
  IL_000a:  ldloc.0
  IL_000b:  ldloca.s   c
  IL_000d:  constrained. [mscorlib]System.Char
  IL_0013:  callvirt   instance string [mscorlib]System.Object::ToString()
  IL_0018:  call       string [mscorlib]System.String::Concat(string,
                                                              string)
  IL_001d:  stloc.0
  IL_001e:  ret
} // end of method Program::Main

I’m no IL expert, but both where the char value is loaded from (at IL_000b), and how it is typed is different. The Win32 IL in VS2015 is exactly the same as the NETMF IL, but the Win32 JIT’er handles this case correctly, and the NETMF interpreter does not, so there may be a fix needed in the NETMF interpreter. Now, that fix MAY have already occurred in the 4.4 code, and if so, I am sure they will say so on the issue that I opened.

Therefore this should affect all NETMF devices so long as the code was compiled under VS2015.

…and now that we’re into this, I do recall a comment somewhere on a different github (4.4) issue about “forcing the use of the old compiler” due to some other aspect of Roslyn that wasn’t handled correctly. I have no idea if they followed through on that, or how that will affect use of 4.3 on VS2015. That is, will there be a new release that is VS2015 safe, or a recommendation not to use VS2015 with 4.3, or some other mitigation? It’s interesting to me because I use VS2015 almost exclusively now and this sort of thing could cause regressions in my existing code.

@ Mr. John Smith - True

BTW, optimized my code with StringBuilder but was have to add #if NETMF41 to support even FEZ Mini that is used in the first version of imBMW in my friend’s car. Yes, FEZ Mini is still alive! :slight_smile: ArrayHelpers optimized with StringBuilder, Char Concat fixed · toxsedyshev/imBMW@045759c · GitHub

Now I’m fixing all char concats in the project. I hope I won’t find some other compiler issues - having both VS2013 and VS2015 on 120gb ssd is a pain.

Well, based on the discussion on github, I think this is going to boil down to either a) do something to make 4.3-based builds on VS2015 use the old compiler, b) don’t use VS2015 for 4.3 builds; or, I guess c) adapt your code to the weirdness, but the weirdness is not easily quantified, so I couldn’t tell you what blanket changes to make.

The problem is that VS2015 and the newer compiler (Roslyn) is emitting at least one IL opcode that is not supported by the NETMF interpreter. The opcode gets ignored and the next instruction ends up invoking the wrong virtual function.

If there’s no fix to the 4.3 interpreter (and no one is actually expecting that there will be - support for this instruction is not even planned for 4.4) then the only solution is a hack to make VS2015 use the older compiler or just avoiding VS2015 with 4.3. I believe (but cannot verify) that the 4.4 templates force the use of the older compiler, because the team detected other issues with Roslyn earlier in the 4.4 effort. Hopefully, they will back-port that fix to 4.3. I’m waiting for an answer on that.

1 Like