FractalFir / rustc_codegen_clr

This rust compiler backend(module) emmits valid CIL (.NET IR), enabling you to use Rust in .NET projects.
MIT License
1.39k stars 30 forks source link

Can't get any existing tests to pass #6

Closed karashiiro closed 11 months ago

karashiiro commented 11 months ago

Hello, I was getting this set up locally to see if I could help with anything, but I haven't been able to get any of the existing tests to pass the ilasm step during cargo test. Is this expected, and if so, what's the process for checking if things work?

Once the IL is built, running ilasm manually produces syntax errors (not sure why the options are being parsed wrong but setting that aside for now; running ilasm /dll /output:vec.dll vec.il produces the same output even though that should be exactly what the test does):

root ➜ /workspaces/rustc_codegen_clr/test (main) $ ilasm /DLL /OUTPUT=vec.dll vec.il

.NET IL Assembler.  Version 7.0.0
C
Assembling '/DLL.il'  to EXE --> '/DLL.exe'
Could not open /DLL.il

Assembling '/OUTPUT=vec.dll'  to EXE --> '/DLL.exe'
Could not open /OUTPUT=vec.dll

Assembling 'vec.il'  to EXE --> '/DLL.exe'
Source file is UTF-8

vec.il(2) : warning : Reference to undeclared extern assembly 'System.Runtime'. Attempting autodetect
vec.il(2) : warning : Non-sealed value class, made sealed
vec.il(5) : warning : Non-sealed value class, made sealed
vec.il(13) : warning : Non-sealed value class, made sealed
vec.il(15) : warning : Nested class has non-nested visibility (0x00000001), changed to nested (0x00000002)
vec.il(15) : warning : Non-sealed value class, made sealed
vec.il(18) : warning : Nested class has non-nested visibility (0x00000001), changed to nested (0x00000002)
vec.il(18) : warning : Non-sealed value class, made sealed
vec.il(26) : warning : Non-sealed value class, made sealed
vec.il(28) : warning : Nested class has non-nested visibility (0x00000001), changed to nested (0x00000002)
vec.il(28) : warning : Non-sealed value class, made sealed
vec.il(32) : warning : Nested class has non-nested visibility (0x00000001), changed to nested (0x00000002)
vec.il(32) : warning : Non-sealed value class, made sealed
vec.il(36) : warning : Nested class has non-nested visibility (0x00000001), changed to nested (0x00000002)
vec.il(36) : warning : Non-sealed value class, made sealed
vec.il(44) : warning : Non-sealed value class, made sealed
vec.il(46) : warning : Nested class has non-nested visibility (0x00000001), changed to nested (0x00000002)
vec.il(46) : warning : Non-sealed value class, made sealed
vec.il(49) : warning : Nested class has non-nested visibility (0x00000001), changed to nested (0x00000002)
vec.il(49) : warning : Non-sealed value class, made sealed
vec.il(52) : warning : Nested class has non-nested visibility (0x00000001), changed to nested (0x00000002)
vec.il(52) : warning : Non-sealed value class, made sealed
vec.il(55) : warning : Nested class has non-nested visibility (0x00000001), changed to nested (0x00000002)
vec.il(55) : warning : Non-sealed value class, made sealed
vec.il(64) : warning : Non-sealed value class, made sealed
vec.il(70) : warning : Non-sealed value class, made sealed
vec.il(74) : error : syntax error at token 'flags' in:  .field public uint32 flags

***** FAILURE ***** 

env info

FractalFir commented 11 months ago

Huh, this may be an issue with other versions of ilasm. I am using mono ilasm 6.12, so there could be some differences in terms of syntax / command arguments. Could you provide vec.il so that I could try and assemble it with my version of ilasm?

FractalFir commented 11 months ago

I have tried the .NET Core version of ilasm and it has this issue. So this stems from differences in syntax accepted by different "flavors" of ilasm.

FractalFir commented 11 months ago

The name flags seems to be reserved in newer versions of ilasm. I will add it to the list of field names to escape.

FractalFir commented 11 months ago

Syntax errors should be fixed in eb7a183. I am still working on the issue with arguments. There seems to be a difference in the way mono ilasm and .NET core ilasm parse arguments.

karashiiro commented 11 months ago

The file, for reference even though it looks like this is being dealt with without it: vec.il.txt (renamed to vec.il.txt so GH accepts it). Ignore the extra valuetype keywords, I was messing with that since ilasm didn't like RustVoid* either. No other local changes besides that, but this issue was happening even before I made any changes.

The specific ilasm I was using just comes from Nuget, ripped straight from the CDN: https://globalcdn.nuget.org/packages/runtime.linux-x64.microsoft.netcore.ildasm.7.0.0.nupkg (unzip can handle nupkgs just fine).

karashiiro commented 11 months ago

Actually that's the ildasm package, ilasm is just https://globalcdn.nuget.org/packages/runtime.linux-x64.microsoft.netcore.ilasm.7.0.0.nupkg though

FractalFir commented 11 months ago

You were on the right track - RustVoid was the cause of one of the issues. The function prefixed_type_cli is supposed to add valuetype prefix to all types that require it, but RustVoid was accidentally left out.

FractalFir commented 11 months ago

The bug involving invalid arguments for ilasm should also now be fixed in 684e8cbbc82cc84e1466f3ce75a9c1c6eb1cba1e.

If everything is working correctly, all tests besides

    compile_test::main
    compile_test::nbody
    compile_test::vec

should run, with main and nbody not compiling, and vec compiling with warnings about unwinding/drops.

If you have any more issues or any questions about the codebase, feel free to ask. Function documentation is very lacking right now.

karashiiro commented 11 months ago

It's looking much better, but about half of the tests are still failing on my end: cargo_test.log

I think this is a simpler issue, dotnet --info is producing different output than what the test is expecting, so it's dumping a multiline string and causing the JSON parser to fail.

An example (add.runtimeconfig.json):

{
            "runtimeOptions": {
              "tfm": "netcoreapp3.1",
              "framework": {
                "name": "Microsoft.NETCore.App",
                "version": "7.0.401
 Commit:    eb26aacfec

Runtime Environment:
 OS Name:     debian
 OS Version:  11
 OS Platform: Linux
 RID:         debian.11-x64
 Base Path:   /usr/share/dotnet/sdk/7.0.401/

Host:
  Version:      7.0.11"
              },
              "configProperties": {
                "System.Threading.ThreadPool.MinThreads": 4,
                "System.Threading.ThreadPool.MaxThreads": 25
              }
            }
          }
karashiiro commented 11 months ago

After fixing that, I get the same test results as you, so I think this can be closed after #7 is merged.